RFR: 8292083: Detected container memory limit may exceed physical machine memory [v3]

[Severin Gehwolf]

On Wed, 17 Aug 2022 13:07:55 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Jonathan Dowland has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   restructure conditions for legibility
>>   
>>    * split assignment to mem_limit from reading it
>>    * nest if expressions to avoid comparing mem_limit twice
>
> src/hotspot/os/linux/os_linux.cpp line 223:
> 
>> 221:   if (OSContainer::is_containerized()) {
>> 222:     jlong mem_limit;
>> 223:     if ((mem_limit = OSContainer::memory_limit_in_bytes()) > 0 && mem_limit < phys_mem) {
> 
> I don't understand this, but also not the preexisting code.
> 
> Did we not just *overwrite* `os::Linux::_physical_memory` during the initialization of the container subsystem depending on the limit? Why is there even a need for added logic here, why not just return `os::Linux::_physical_memory`?
> 
> Maybe the answer is that `os::physical_memory()` is supposed to mirror cgroup limit changes during the VMs lifetime (is it?). But then, the question is the other way around, why bother correcting the value during initialization if we need to correct it in `os::physical_memory()` ?

Good questions. I think the answer is that the cgroups values might change during VMs lifetime. I never understood why that was a goal to support, but fine. It's possible to do via docker so I guess that's why. As to the question why bother doing it in init too? I don't know. Maybe @dholmes-ora remembers?

Perhaps the better fix would be to fix the values in `OSContainer::memory_limit_in_bytes()`, instead of at all call-sites. It appears `os::Linux::available_memory()` would need similar treatment.

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

PR: https://git.openjdk.org/jdk/pull/9880