RFR: JDK-8321266: Add diagnostic RSS threshold [v2]

[Ashutosh Mehra]

On Tue, 5 Dec 2023 10:36:29 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Hi Thomas,
>> 
>> I've taken a first pass through this and it seems okay in principle. A number of initial comments/suggestions below.
>> 
>> Thanks.
>
>> Hi Thomas,
>> 
>> I've taken a first pass through this and it seems okay in principle. A number of initial comments/suggestions below.
>> 
>> Thanks.
> 
> Thanks a lot, David!
> 
> Makes me happy to see this finds acceptance at least in principle.
> 
> I changed:
> - get_rss to get_RSS
> - removed the "0 means off" text, since I assume passing 0 would be likely a user error. Instead, I also added an error check for percentage = 0.0.
> - added a warning if the OS does not support this feature

Hi @tstuefe this looks useful feature and seems to provides a way to deal with OOM killer in containers. If the user has set container memory limit to 256MB, then the RssLimit can be set to around 200MB. This would let the JVM catch the OOM before it is handled by the kernel.
But I have one concern. The effectiveness of this solution really depends on how frequently the check is done. If there is a sudden memory spike, it should, ideally, last longer than `RssLimitCheckInterval` for RssWatcher to take the action. Flipping it the other way, we can say RssWatcher can catch memory spikes that last longer than `RssLimitCheckInterval`.
Even then, it can catch the spike only as long as it is less than the container limit.
This raises the question of determining the effective value of `RssLimit` and `RssLimitCheckInterval`. For instance, compilations can induce memory spike which may last for few hundred milliseconds at the most, which is much lesser than the default value of  5 secs for `RssLimitCheckInterval`.
What are your thoughts on this?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16938#issuecomment-1841141950