[jdk8u-dev] RFR: 8239559: Cgroups: Incorrect detection logic on some systems [v4]
Severin Gehwolf
sgehwolf at openjdk.org
Mon Nov 7 17:35:38 UTC 2022
On Fri, 4 Nov 2022 10:34:11 GMT, Jonathan Dowland <jdowland at openjdk.org> wrote:
>> This is a backport of 53ee0c4963007b86db7979312b81f990e6ce271a via the 11u backport 53ee0c4963007b86db7979312b81f990e6ce271a for cgroups v2 support in jdk8u-dev.
>>
>> It isn't clean: context differences in CgroupSubsystemFactory.java around the use of post-8u logging in the original patch.
>>
>> I also had to Optional.isEmpty which is not present in 8u's Optional.
>
> Jonathan Dowland has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 58 commits:
>
> - replace post-jdk8u Optional.isEmpty
> - 8239559: Cgroups: Incorrect detection logic on some systems
>
> Adjust heuristic with cgroup mounts according to mountinfo
>
> Backport-of: 53ee0c4963007b86db7979312b81f990e6ce271a
> - 8240189: [TESTBUG] Some cgroup tests are failing after JDK-8231111
>
> Reviewed-by: mdoerr
> Backport-of: c92adf41587767e9c5c8e116cfaeb375d36928aa
> - Don't pass --add-exports to jdk8u java inside docker
>
> These are JDK9+ module specific parameters.
> - 8244500: jtreg test error in test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
>
> When the kernel doesn't support swap limits, expect host values instead.
>
> Reviewed-by: sgehwolf
> Backport-of: fb5132254d834ba01a4b65ce64143843e83c674e
> - Move test to a more 8u-appropriate location
>
> Container (cgroups, docker) tests in 8u reside in
> hotspot/test/runtime/containers
> - remove duplicate include of osContainer_linux
>
> With the backport of 8189762, the include of osContainer_linux was
> moved to a later #ifdef stanza, relative to the original. This caused
> a context problem with this backport.
> - 8239785: Cgroups: Incorrect detection logic on old systems in hotspot
>
> Return NULL subsystem if no cgroup controllers are mounted.
>
> Reviewed-by: sgehwolf
> Backport-of: add18914fb1294999877b563c734a25b4c17b922
> - 8237479: 8230305 causes slowdebug build failure
>
> Declare methods as pure virtual.
>
> Backport-of: 4ca06995855b5c974321d7b3622d661b8d27ba76
> - 8253714: [cgroups v2] Soft memory limit incorrectly using memory.high
>
> The early implementation of cgroups v2 support was done with
> crun 0.8 and it contained a bug which set memory.high over
> memory.low when --memory-reservation was being used as a CLI
> option.
>
> This bug has been fixed in later crun versions, starting with
> crun 0.11. Use memory.low in OpenJDK as well.
>
> Backport-of: ff6843ca4842498791061f924c545fa9469cc1dc
> - ... and 48 more: https://git.openjdk.org/jdk8u-dev/compare/f0ac3199...45ec61df
Looks good other than the test change that's necessary.
jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java line 47:
> 45: * @requires os.family == "linux"
> 46: * @modules java.base/jdk.internal.platform
> 47: * @library /test/lib
This test also doesn't run on JDK 8u. I had to use this patch:
commit 3b4140fac83f628d15fee8e8044480a2692f3a24
Author: Severin Gehwolf <sgehwolf at redhat.com>
Date: Mon Nov 7 18:30:39 2022 +0100
Fix test for JDK 8u
diff --git a/jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java b/jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java
index 6f47ea0602..dfec2af0f1 100644
--- a/jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java
+++ b/jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java
@@ -36,15 +36,15 @@ import org.junit.Test;
import jdk.internal.platform.CgroupSubsystemFactory;
import jdk.internal.platform.CgroupSubsystemFactory.CgroupTypeResult;
-import jdk.test.lib.Utils;
-import jdk.test.lib.util.FileUtils;
+import jdk.testlibrary.Utils;
+import jdk.testlibrary.FileUtils;
/*
* @test
* @requires os.family == "linux"
* @modules java.base/jdk.internal.platform
- * @library /test/lib
+ * @library /lib/testlibrary
* @run junit/othervm TestCgroupSubsystemFactory
*/
public class TestCgroupSubsystemFactory {
@@ -104,20 +104,20 @@ public class TestCgroupSubsystemFactory {
try {
existingDirectory = Utils.createTempDirectory(TestCgroupSubsystemFactory.class.getSimpleName());
Path cgroupsZero = Paths.get(existingDirectory.toString(), "cgroups_zero");
- Files.writeString(cgroupsZero, cgroupsZeroHierarchy, StandardCharsets.UTF_8);
+ Files.write(cgroupsZero, cgroupsZeroHierarchy.getBytes(StandardCharsets.UTF_8));
cgroupv1CgInfoZeroHierarchy = cgroupsZero;
cgroupv2CgInfoZeroHierarchy = cgroupsZero;
cgroupv1MntInfoZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_empty");
- Files.writeString(cgroupv1MntInfoZeroHierarchy, mntInfoEmpty);
+ Files.write(cgroupv1MntInfoZeroHierarchy, mntInfoEmpty.getBytes());
cgroupv2MntInfoZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv2");
- Files.writeString(cgroupv2MntInfoZeroHierarchy, mntInfoCgroupsV2Only);
+ Files.write(cgroupv2MntInfoZeroHierarchy, mntInfoCgroupsV2Only.getBytes());
cgroupv1CgInfoNonZeroHierarchy = Paths.get(existingDirectory.toString(), "cgroups_non_zero");
- Files.writeString(cgroupv1CgInfoNonZeroHierarchy, cgroupsNonZeroHierarchy);
+ Files.write(cgroupv1CgInfoNonZeroHierarchy, cgroupsNonZeroHierarchy.getBytes());
cgroupv1MntInfoNonZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_non_zero");
- Files.writeString(cgroupv1MntInfoNonZeroHierarchy, mntInfoHybrid);
+ Files.write(cgroupv1MntInfoNonZeroHierarchy, mntInfoHybrid.getBytes());
} catch (IOException e) {
throw new RuntimeException(e);
}
@@ -149,7 +149,7 @@ public class TestCgroupSubsystemFactory {
String mountInfo = cgroupv1MntInfoZeroHierarchy.toString();
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups);
- assertTrue("zero hierarchy ids with no mounted controllers => empty result", result.isEmpty());
+ assertTrue("zero hierarchy ids with no mounted controllers => empty result", Optional.empty().equals(result));
}
@Test
diff --git a/jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java b/jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java
index 5979815d41..31bfeea36c 100644
--- a/jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java
+++ b/jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java
@@ -38,7 +38,11 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Arrays;
import java.util.LinkedList;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.Paths;
+import java.nio.file.attribute.FileAttribute;
+
import java.util.Collections;
import java.util.Objects;
import java.util.regex.Pattern;
@@ -440,4 +444,24 @@ public final class Utils {
public static String getTestSourcePath(String fileName) {
return Paths.get(System.getProperty("test.src")).resolve(fileName).toString();
}
+
+ /**
+ * Creates an empty directory in "user.dir" or "."
+ * <p>
+ * This method is meant as a replacement for {@code Files#createTempDirectory(String, String, FileAttribute...)}
+ * that doesn't leave files behind in /tmp directory of the test machine
+ * <p>
+ * If the property "user.dir" is not set, "." will be used.
+ *
+ * @param prefix
+ * @param attrs
+ * @return the path to the newly created directory
+ * @throws IOException
+ *
+ * @see {@link Files#createTempDirectory(String, String, FileAttribute...)}
+ */
+ public static Path createTempDirectory(String prefix, FileAttribute<?>... attrs) throws IOException {
+ Path dir = Paths.get(System.getProperty("user.dir", "."));
+ return Files.createTempDirectory(dir, prefix);
+ }
}
-------------
Changes requested by sgehwolf (Reviewer).
PR: https://git.openjdk.org/jdk8u-dev/pull/136
More information about the jdk8u-dev
mailing list