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

[Severin Gehwolf]

On Thu, 25 Aug 2022 16:26:17 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Hmm, I get uncomfortable if APIs mix layers and try to be smart.
> 
> As in this case, an API that claims to return CG mem limit but mixes in knowledge about the total physical memory. Then we have os::Linux::available_memory() in turn reporting CG mem limit instead if we are containerized. It is getting hard to understand who reports what under which conditions.
> 
> In general, I prefer dumb APIs that do exactly what they are named for, and some arbitration layer atop of it resolving conflicts and deciding whose values to use. Makes the code much easier to understand.

All fair points, but this patch doesn't change the status quo. For example, `CgroupSubsystem::active_processor_count()` uses `os::Linux::active_processor_count()` to bound the cores reported to the JVM above by the physical cores (or virtualized cores in a VM). Similarly, `CgroupV1Subsystem` uses ` _unlimited_memory = (LONG_MAX / os::vm_page_size()) * os::vm_page_size();` Again, OS code. The latter, I believe, since day one of the container support code.

I guess it would be conceivable to move the "arbitration" to `OSContainer` dealing with limits exceeding physical memory. It could either pass upper bounds down to the CG layer or let it do it's thing first and do the sanity checking after. Would something like that be more amenable?

The gist of this patch is code like this:


jlong CgroupV1Subsystem::read_memory_limit_in_bytes() {
  GET_CONTAINER_INFO(julong, _memory->controller(), "/memory.limit_in_bytes",
                     "Memory Limit is: " JULONG_FORMAT, JULONG_FORMAT, memlimit);
  if (memlimit >= _unlimited_memory) {
   ...
  } else {
    return (jlong)memlimit;
  }


... might return arbitrary large values on some systems (note that `_unlimited_memory = (LONG_MAX / os::vm_page_size()) * os::vm_page_size()` which can be fairly large; we've observed systems where the cgroup interface files have `92233720365056` whereas the `_unlimited_memory` value was `9223372036854771712`). That on a system with 8GB physical memory.
 
> That does not mean that I want a rewrite of this patch. I'm just not sure this is the right direction. I have to think this through some more.

OK.

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

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