RFR: 8292083: Detected container memory limit may exceed physical machine memory

[Severin Gehwolf]

On Mon, 15 Aug 2022 14:51:51 GMT, Jonathan Dowland <jdowland at openjdk.org> wrote:

> We discovered some systems configured with cgroups v1 which report a bogus container memory limit value which is above the physical memory of the host. OpenJDK then calculates flags such as InitialHeapSize based on this invalid value; this can be larger than the available memory which can result in the OS terminating the process due to OOM.
> 
> hotspot's container awareness attempts to sanity check the limit value by ensuring it's below `_unlimited_memory = (LONG_MAX / os::vm_page_size()) * os::vm_page_size()`, but that still leaves a large range of potential invalid values between physical RAM and that ceiling value.
> 
> Cgroups V1 in particular returns an uninitialised value for the memory limit when one has not been explicitly set. Cgroups v2 does not suffer the same problem: however, it's possible for any value to be set for the max memory, including values exceeding the available physical memory, in either v1 or v2.
> 
> This fixes the problem in two places. Further work may be required in the area of Java metrics / MXBeans. I'd also look again at whether the existing ceiling value `_unlimited_memory` serves any useful purpose. I personally don't feel those improvements should hold up this fix.

This looks mostly good. I'd prefer if we changed the test to not rely on `InitialHeapSize` as that might get ergonomically set.

src/hotspot/os/linux/osContainer_linux.cpp line 71:

> 69:     os::Linux::set_physical_memory(mem_limit);
> 70:     log_info(os, container)("Memory Limit is: " JLONG_FORMAT, mem_limit);
> 71:   } else if (mem_limit >= host_memory) {

We should probably change this to `else if (log_is_enabled(Trace, os, container) && mem_limit >= host_memory) {...` as we only do trace logs in that branch.

src/hotspot/os/linux/os_linux.cpp line 228:

> 226:     }
> 227:     log_debug(os, container)("container memory limit %s: " JLONG_FORMAT ", using host value",
> 228:                             mem_limit == OSCONTAINER_ERROR ? "failed" : "unlimited", mem_limit);

These log lines now result in messages like:


[0.037s][debug][os,container] container memory limit unlimited: 38654705664, using host value


It looks a bit strange, so perhaps we should account for the exceeding-host-memory case. Thoughts?

test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java line 109:

> 107:     // set java's InitialRAMPercentage to 25%, and check the calculated InitialHeapSize
> 108:     // to see if that was calculated relative to P (P/4) or 2P (P/2).
> 109:     private static void testBadMemoryLimit()

Could we give this test a better name? How about `testContainerMemExceedsPhysical`? Anyway, we could perhaps write a `WhiteBox` based one which asserts that the `os::physical_memory()` never exceeds the host memory. See `WhiteBox.printOsInfo()` for an example as to how that's done.

As to the code-path in `OSContainer::init` we could assert some log lines with `-Xlog:os+container=trace`. For example for the `Container memory limit exceeded or equal to physical memory!` line to be present. Together with the `os::physical_memory()` whitebox test this should cover all bases. Thoughts?

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

Changes requested by sgehwolf (Reviewer).

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