[jdk8u-dev] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy [v3]

[Severin Gehwolf]

On Mon, 26 Sep 2022 10:13:25 GMT, Jonathan Dowland <jdowland at openjdk.org> wrote:

>> This is a backport of [4def210a22faaec6b47912dd314e6365ea48d28f](https://github.com/openjdk/jdk/commit/4def210a22faaec6b47912dd314e6365ea48d28f) for jdk8u-dev as part of an effort to backport cgroups v2 support.
>> 
>> It does not apply clean. Paths need unshuffling. A number of changes were needed for 8u support. I've structured the PR as separate commits, with each change made in a separate commit for (hopefully) ease of review.
>> 
>> Not all the new tests pass:     TestDockerMemoryMetrics  failing one, specifically:
>> 
>>     Exception in thread "main" java.lang.RuntimeException: Memory and swap limit not equal, expected : [209715200, 1073741824], got : [209715200, 864026624]
>> 
>> I think this is fixed in a further patch to backport, and will confirm.
>
> Jonathan Dowland has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
> 
>  - TestCgroupSubsystemController: rework use of Files.writeString
>  - CgroupSubsystemController: fix library paths
>    
>    We need the testlibrary copy of FileUtils but the test.lib.util copy of
>    Utils (method createTempDirectory is missing from the testlib copy)
>  - TestCgroupSubsystemController: fix jtreg @library path
>  - Replace Arrays.compare with Arrays.equals
>    
>    jdk8u does not have Arrays.compare()
>  - incorporate (part of) 8275713: TestDockerMemoryMetrics test fails on recent runc
>    
>    The main hunk from 8275713 was rolled up in the changes for 8231111.
>    This line is also necessary.
>  - update mapfile for new JNI method name
>  - tests for the backport
>    
>    two files had fairly significant merge conflicts, eyeball only to resolve
>    
>    haven't run any of them yet -- will depend on the rest of the patch
>  - CgroupSubsystemFactory: remove logging lines
>    
>    jdk8u doesn't have java.lang.System.Logger. There's no existing logging
>    in place for other classes in the Metrics/platform/etc family that I can
>    see, so remove it.
>  - Metrics => CgroupV1Subsystem.java
>    
>    unshuffle, rename, resolve unclean hunks although I'm not sure why they
>    didn't apply, they look trivial and the context is the same, will look
>    closer
>  - Metrics.java: easy commit, mostly doc-only hunks
>  - ... and 5 more: https://git.openjdk.org/jdk8u-dev/compare/91dc4a09...0ea3c608

Looks mostly good. A couple of suggestions.

jdk/test/jdk/internal/platform/docker/MetricsMemoryTester.java line 32:

> 30: 
> 31:     private static final long UNLIMITED = -1;
> 32: 

This hunk is part of JDK-8275713, which gets backported as part of this patch. Please do `/issue add JDK-8275713` so it gets a record of it being backported in JBS.

jdk/test/jdk/internal/platform/docker/MetricsMemoryTester.java line 133:

> 131:             // limit back.
> 132:             if (kmemlimit != UNLIMITED && limit != kmemlimit) {
> 133:                 throw new RuntimeException("Kernel Memory limit not equal, expected : ["

Also code from JDK-8275713.

jdk/test/lib/jdk/test/lib/containers/cgroup/CgroupMetricsTester.java line 56:

> 54:             if (b.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) > 0) {
> 55:                 return overflowRetval;
> 56:             }

This is code from JDK-8228585 which wasn't previously part of 8u. Please add `/issue add JDK-8228585` to this backport since this backport will bring code of that bug with it.

jdk/test/lib/jdk/test/lib/containers/cgroup/MetricsTesterCgroupV1.java line 424:

> 422:         Integer[] newVal = CgroupMetricsTester.convertCpuSetsToArray(cpusstr);
> 423:         Arrays.sort(newVal);
> 424:         if (! Arrays.equals(oldVal, newVal)) {

Style nit: `!Arrays.equals(..)` over `! Arrays.equals(...)`. Drop the space. This repeats a couple of times in this file.

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

PR: https://git.openjdk.org/jdk8u-dev/pull/121