On Fri, 2019-03-22 at 14:25 -0400, Bob Vandette wrote:
> Is there ever a situation where the memory.limit_in_bytes could be unlimited
> but the
> hierarchical_memory_limit is not?
Yes. This is the very bug. Under a systemd slice with newer kernels the
memory.limit_in_bytes file OpenJDK looks at has unlimited, but
memory.stat on the same hierarchy has the correct setting.
Patched JDK with -Xlog:os+container=trace
[0.051s][trace][os,container] Path to /memory.limit_in_bytes is
/sys/fs/cgroup/memory/user.slice/user-cg.slice/run-raf53fa46d5cc47ce857fbfaae04459c1.scope/memory.limit_in_bytes
[0.051s][trace][os,container] Memory Limit is: 9223372036854771712
[0.051s][trace][os,container] Non-Hierarchical Memory Limit is: Unlimited
[0.051s][trace][os,container] Path to /memory.stat is
/sys/fs/cgroup/memory/user.slice/user-cg.slice/run-raf53fa46d5cc47ce857fbfaae04459c1.scope/memory.stat
[0.051s][trace][os,container] Hierarchical Memory Limit is: 10485760
$ cat
/sys/fs/cgroup/memory/user.slice/user-cg.slice/run-raf53fa46d5cc47ce857fbfaae04459c1.scope/memory.limit_in_bytes
9223372036854771712
$ cat /sys/fs/cgroup/memory/user.slice/user-cg.slice/memory.limit_in_bytes
10485760
$ grep hierarchical
/sys/fs/cgroup/memory/user.slice/user-cg.slice/run-raf53fa46d5cc47ce857fbfaae04459c1.scope/memory.stat
hierarchical_memory_limit 10485760
hierarchical_memsw_limit 9223372036854771712
This was mentioned in the bug comments as well.
> Could you maybe combine subsystem_file_contents with
> subsystem_file_line_contents
> by adding an additional argument?
Sure, I could look into that. I'll post an updated webrev soon.
Thanks for the review!
Cheers,
Severin
> BTW: I found another problem with the mountinfo/cgroup parsing that impacts
> the
> container tests.
>
> I don’t know why it only caused a failure on one of my systems. I’m going to
> file another bug. You might want to test with these changes to make sure
> your looking at the correct subsystem files.
OK. Thanks for the heads-up. It looks unrelated, though.
>
> diff --git
> a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
> b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
> ---
> a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
> +++
> b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
> @@ -60,7 +60,7 @@
> path = mountPoint;
> }
> else {
> - if (root.indexOf(cgroupPath) == 0) {
> + if (cgroupPath.indexOf(root) == 0) {
> if (cgroupPath.length() > root.length()) {
> String cgroupSubstr =
> cgroupPath.substring(root.length());
> path = mountPoint + cgroupSubstr;
> diff --git a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
> b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
> --- a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
> +++ b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
> @@ -85,7 +85,7 @@
> String mountPoint = paths[1];
> if (root != null && cgroupPath != null) {
> if (root.equals("/")) {
> - if (cgroupPath.equals("/")) {
> + if (!cgroupPath.equals("/")) {
> finalPath = mountPoint + cgroupPath;
> } else {
> finalPath = mountPoint;
> @@ -94,7 +94,7 @@
> if (root.equals(cgroupPath)) {
> finalPath = mountPoint;
> } else {
> - if (root.indexOf(cgroupPath) == 0) {
> + if (cgroupPath.indexOf(root) == 0) {
> if (cgroupPath.length() > root.length()) {
> String cgroupSubstr =
> cgroupPath.substring(root.length());
> finalPath = mountPoint + cgroupSubstr;
> @@ -103,7 +103,7 @@
> }
> }
> }
> - subSystemPaths.put(subSystem, new String[]{finalPath});
> + subSystemPaths.put(subSystem, new String[]{finalPath,
> mountPoint});
> }
> }
> }
>
>
> Bob.
>
>
>
> > On Mar 22, 2019, at 6:43 AM, Severin Gehwolf <sgehw...@redhat.com> wrote:
> >
> > Hi,
> >
> > Please review this change which improves container detection support
> > tin the JVM. While docker container detection works quite well the
> > results for systemd slices with memory limits are mixed and depend on
> > the Linux kernel version in use. With newer kernel versions becoming
> > more widely used we should improve JVMs memory limit detection support
> > as well. This should be entirely backwards compatible as the
> > hierarchical limit will only be used if everything else is unlimited.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8217338
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217338/04/webrev/
> >
> > Testing: Manual testing of -XshowSettings and -Xlog:os+container=trace
> > with systemd slices on affected Linux distributions: Fedora 29,
> > recent Ubuntu (18-10). Existing docker tests pass. I'm currently also
> > running this through jdk/submit.
> >
> > Thoughts?
> >
> > Thanks,
> > Severin
> >
