RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v11]

Sergey Chernyshev schernyshev at openjdk.org
Fri Feb 21 19:56:17 UTC 2025


On Mon, 17 Feb 2025 11:28:38 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Sergey Chernyshev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   added details in the log message
>
> test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java line 60:
> 
>> 58: 
>> 59:             Common.prepareWhiteBox();
>> 60:             DockerTestUtils.buildJdkContainerImage(imageName);
> 
> This can be moved out of the condition. It's there in both cases.

Done

> test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java line 85:
> 
>> 83:         } else {
>> 84:             System.out.println("Metrics are from neither cgroup v1 nor v2, skipped for now.");
>> 85:         }
> 
> Suggestion:
> 
>         throw new RuntimeException("cgroup version not known? was: " + metrics.getProvider());

I made it with `jtreg.SkippedException`

> test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java line 98:
> 
>> 96:         opts.addDockerOpts("--privileged")
>> 97:             .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host"))
>> 98:             .addDockerOpts("--memory", "1g");
> 
> Please extract this "larger" memory limit out of this function. The expectation is to have:
> 
> 
> Container memory limit => X
> Some lower limit in a moved path => Y
> 
> Expect that the lower limit of Y is being detected.
> 
> 
> For example:
> 
> 
> testMemoryLimitSubgroupV1("1g", "100m", "104857600", false);

done.

> test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java line 118:
> 
>> 116:         opts.addDockerOpts("--privileged")
>> 117:             .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host"))
>> 118:             .addDockerOpts("--memory", "1g");
> 
> Same here with the `1g` container memory limit. Move it to a parameter.

done. extracted the parameter.

> test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetricsSubgroup.java line 72:
> 
>> 70:         } else {
>> 71:             System.out.println("Metrics are from neither cgroup v1 nor v2, skipped for now.");
>> 72:         }
> 
> Please `throw` here.

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1966094564
PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1966094301
PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1966093295
PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1966092984
PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1966092288


More information about the core-libs-dev mailing list