Skip to content

Commit

Permalink
8293540: [Metrics] Incorrectly detected resource limits with addition…
Browse files Browse the repository at this point in the history 
…al cgroup fs mounts

Backport-of: 6d83482a6b5f1898514fd450d8143dbfef57e362
  • Loading branch information
@jerboaa
jerboaa committed Nov 18, 2022
1 parent 7706fa8 commit 2a3eea0
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 38 deletions.
44 changes: 19 additions & 25 deletions src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
View file
Expand Up @@ -320,30 +320,11 @@ private static boolean amendCgroupInfos(String mntInfoLine,
case MEMORY_CTRL: // fall-through
case CPU_CTRL:
case CPUACCT_CTRL:
case CPUSET_CTRL:
case PIDS_CTRL:
case BLKIO_CTRL: {
CgroupInfo info = infos.get(controllerName);
assert info.getMountPoint() == null;
assert info.getMountRoot() == null;
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
cgroupv1ControllerFound = true;
break;
}
case CPUSET_CTRL: {
CgroupInfo info = infos.get(controllerName);
if (info.getMountPoint() != null) {
// On some systems duplicate cpuset controllers get mounted in addition to
// the main cgroup controllers most likely under /sys/fs/cgroup. In that
// case pick the one under /sys/fs/cgroup and discard others.
if (!info.getMountPoint().startsWith("/sys/fs/cgroup")) {
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
}
} else {
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
}
setMountPoints(info, mountPath, mountRoot);
cgroupv1ControllerFound = true;
break;
}
Expand All @@ -357,10 +338,7 @@ private static boolean amendCgroupInfos(String mntInfoLine,
// All controllers have the same mount point and root mount
// for unified hierarchy.
for (CgroupInfo info: infos.values()) {
assert info.getMountPoint() == null;
assert info.getMountRoot() == null;
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
setMountPoints(info, mountPath, mountRoot);
}
}
cgroupv2ControllerFound = true;
Expand All @@ -369,6 +347,22 @@ private static boolean amendCgroupInfos(String mntInfoLine,
return cgroupv1ControllerFound || cgroupv2ControllerFound;
}

private static void setMountPoints(CgroupInfo info, String mountPath, String mountRoot) {
if (info.getMountPoint() != null) {
// On some systems duplicate controllers get mounted in addition to
// the main cgroup controllers (which are under /sys/fs/cgroup). In that
// case pick the main one and discard others as the limits
// are associated with the ones in /sys/fs/cgroup.
if (!info.getMountPoint().startsWith("/sys/fs/cgroup")) {
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
}
} else {
info.setMountPoint(mountPath);
info.setMountRoot(mountRoot);
}
}

public static final class CgroupTypeResult {
private final boolean isCgroupV2;
private final boolean anyControllersEnabled;
Expand Down
15 changes: 15 additions & 0 deletions test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
View file
Expand Up @@ -87,6 +87,11 @@ public static void main(String[] args) throws Exception {
"1G", Integer.toString(((int) Math.pow(2, 20)) * 1024),
"1500M", Integer.toString(((int) Math.pow(2, 20)) * (1500 - 1024))
);
testOperatingSystemMXBeanAwareness(
"100M", Integer.toString(((int) Math.pow(2, 20)) * 100),
"200M", Integer.toString(((int) Math.pow(2, 20)) * (200 - 100)),
true /* additional cgroup fs mounts */
);
final String hostMaxMem = getHostMaxMemory();
testOperatingSystemMXBeanIgnoresMemLimitExceedingPhysicalMemory(hostMaxMem);
testMetricsIgnoresMemLimitExceedingPhysicalMemory(hostMaxMem);
Expand Down Expand Up @@ -171,6 +176,12 @@ private static void testOOM(String dockerMemLimit, int sizeToAllocInMb) throws E

private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
String swapAllocation, String expectedSwap) throws Exception {
testOperatingSystemMXBeanAwareness(memoryAllocation, expectedMemory, swapAllocation, expectedSwap, false);
}

private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
String swapAllocation, String expectedSwap, boolean addCgroupMounts) throws Exception {

Common.logNewTestCase("Check OperatingSystemMXBean");

DockerRunOptions opts = Common.newOpts(imageName, "CheckOperatingSystemMXBean")
Expand All @@ -182,6 +193,10 @@ private static void testOperatingSystemMXBeanAwareness(String memoryAllocation,
// diagnostics
.addJavaOpts("--add-exports")
.addJavaOpts("java.base/jdk.internal.platform=ALL-UNNAMED");
if (addCgroupMounts) {
// Extra cgroup mount should be ignored by product code
opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
}

OutputAnalyzer out = DockerTestUtils.dockerRunJava(opts);
out.shouldHaveExitValue(0)
Expand Down
37 changes: 24 additions & 13 deletions test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java
View file
Expand Up @@ -51,7 +51,7 @@

/*
* @test
* @bug 8287107 8287073
* @bug 8287107 8287073 8293540
* @key cgroups
* @requires os.family == "linux"
* @modules java.base/jdk.internal.platform
Expand All @@ -72,8 +72,8 @@ public class TestCgroupSubsystemFactory {
private Path cgroupv1CgInfoNonZeroHierarchy;
private Path cgroupv1MntInfoNonZeroHierarchy;
private Path cgroupv1MntInfoSystemdOnly;
private Path cgroupv1MntInfoDoubleCpusets;
private Path cgroupv1MntInfoDoubleCpusets2;
private Path cgroupv1MntInfoDoubleControllers;
private Path cgroupv1MntInfoDoubleControllers2;
private Path cgroupv1MntInfoColonsHierarchy;
private Path cgroupv1SelfCgroup;
private Path cgroupv1SelfColons;
Expand Down Expand Up @@ -194,9 +194,13 @@ public class TestCgroupSubsystemFactory {
private String mntInfoCgroupsV1SystemdOnly =
"35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup systemd rw,name=systemd\n" +
"26 18 0:19 / /sys/fs/cgroup rw,relatime - tmpfs none rw,size=4k,mode=755\n";
private String mntInfoCgroupv1MoreCpusetLine = "121 32 0:37 / /cpuset rw,relatime shared:69 - cgroup none rw,cpuset\n";
private String mntInfoCgroupsV1DoubleCpuset = mntInfoHybrid + mntInfoCgroupv1MoreCpusetLine;
private String mntInfoCgroupsV1DoubleCpuset2 = mntInfoCgroupv1MoreCpusetLine + mntInfoHybrid;
private String mntInfoCgroupv1MoreControllers = "121 32 0:37 / /cpuset rw,relatime shared:69 - cgroup none rw,cpuset\n" +
"35 30 0:31 / /cgroup-in/memory rw,nosuid,nodev,noexec,relatime shared:7 - cgroup none rw,seclabel,memory\n" +
"36 30 0:32 / /cgroup-in/pids rw,nosuid,nodev,noexec,relatime shared:8 - cgroup none rw,seclabel,pids\n" +
"40 30 0:36 / /cgroup-in/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:12 - cgroup none rw,seclabel,cpu,cpuacct\n" +
"40 30 0:36 / /cgroup-in/blkio rw,nosuid,nodev,noexec,relatime shared:12 - cgroup none rw,seclabel,blkio\n";
private String mntInfoCgroupsV1DoubleControllers = mntInfoHybrid + mntInfoCgroupv1MoreControllers;
private String mntInfoCgroupsV1DoubleControllers2 = mntInfoCgroupv1MoreControllers + mntInfoHybrid;
private String cgroupv1SelfCgroupContent = "11:memory:/user.slice/user-1000.slice/user@1000.service\n" +
"10:hugetlb:/\n" +
"9:cpuset:/\n" +
Expand Down Expand Up @@ -275,11 +279,11 @@ public void setup() {
cgroupv1MntInfoSystemdOnly = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_systemd_only");
Files.writeString(cgroupv1MntInfoSystemdOnly, mntInfoCgroupsV1SystemdOnly);

cgroupv1MntInfoDoubleCpusets = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_cpuset");
Files.writeString(cgroupv1MntInfoDoubleCpusets, mntInfoCgroupsV1DoubleCpuset);
cgroupv1MntInfoDoubleControllers = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_controllers");
Files.writeString(cgroupv1MntInfoDoubleControllers, mntInfoCgroupsV1DoubleControllers);

cgroupv1MntInfoDoubleCpusets2 = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_cpuset2");
Files.writeString(cgroupv1MntInfoDoubleCpusets2, mntInfoCgroupsV1DoubleCpuset2);
cgroupv1MntInfoDoubleControllers2 = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_controllers2");
Files.writeString(cgroupv1MntInfoDoubleControllers2, mntInfoCgroupsV1DoubleControllers2);

cgroupv1CgroupsJoinControllers = Paths.get(existingDirectory.toString(), "cgroups_cgv1_join_controllers");
Files.writeString(cgroupv1CgroupsJoinControllers, cgroupsNonZeroJoinControllers);
Expand Down Expand Up @@ -390,11 +394,11 @@ public void testCgroupv1SystemdOnly() throws IOException {

@Test
public void testCgroupv1MultipleCpusetMounts() throws IOException {
doMultipleCpusetMountsTest(cgroupv1MntInfoDoubleCpusets);
doMultipleCpusetMountsTest(cgroupv1MntInfoDoubleCpusets2);
doMultipleMountsTest(cgroupv1MntInfoDoubleControllers);
doMultipleMountsTest(cgroupv1MntInfoDoubleControllers2);
}

private void doMultipleCpusetMountsTest(Path info) throws IOException {
private void doMultipleMountsTest(Path info) throws IOException {
String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString();
String mountInfo = info.toString();
String selfCgroup = cgroupv1SelfCgroup.toString();
Expand All @@ -406,6 +410,13 @@ private void doMultipleCpusetMountsTest(Path info) throws IOException {
CgroupInfo cpuSetInfo = res.getInfos().get("cpuset");
assertEquals("/sys/fs/cgroup/cpuset", cpuSetInfo.getMountPoint());
assertEquals("/", cpuSetInfo.getMountRoot());
// Ensure controllers at /sys/fs/cgroup will be used
String[] ctrlNames = new String[] { "memory", "cpu", "cpuacct", "blkio", "pids" };
for (int i = 0; i < ctrlNames.length; i++) {
CgroupInfo cinfo = res.getInfos().get(ctrlNames[i]);
assertTrue(cinfo.getMountPoint().startsWith("/sys/fs/cgroup/"));
assertEquals("/", cinfo.getMountRoot());
}
}

@Test
Expand Down
72 changes: 72 additions & 0 deletions test/jdk/jdk/internal/platform/docker/TestDockerBasic.java
View file
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2022, Red Hat, Inc.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8293540
* @summary Verify that -XshowSettings:system works
* @requires docker.support
* @library /test/lib
* @run main/timeout=360 TestDockerBasic
*/

import jdk.test.lib.Utils;
import jdk.test.lib.containers.docker.Common;
import jdk.test.lib.containers.docker.DockerRunOptions;
import jdk.test.lib.containers.docker.DockerTestUtils;

public class TestDockerBasic {
private static final String imageName = Common.imageName("javaDockerBasic");

public static void main(String[] args) throws Exception {
if (!DockerTestUtils.canTestDocker()) {
return;
}

DockerTestUtils.buildJdkContainerImage(imageName);

try {
testXshowSettingsSystem(true);
testXshowSettingsSystem(false);
} finally {
DockerTestUtils.removeDockerImage(imageName);
}
}

private static void testXshowSettingsSystem(boolean addCgroupMounts) throws Exception {
String testMsg = (addCgroupMounts ? " with " : " without ") + " additional cgroup FS mounts in /cgroup-in";
Common.logNewTestCase("Test TestDockerBasic " + testMsg);
DockerRunOptions opts =
new DockerRunOptions(imageName, "/jdk/bin/java", "-version");
opts.addJavaOpts("-esa");
opts.addJavaOpts("-XshowSettings:system");
opts.addDockerOpts("--memory", "300m");
if (addCgroupMounts) {
// Extra cgroup mount should be ignored by product code
opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
}
DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0)
.shouldNotContain("AssertionError")
.shouldContain("Memory Limit: 300.00M");
}
}
18 changes: 18 additions & 0 deletions test/jdk/jdk/internal/platform/docker/TestDockerCpuMetrics.java
View file
Expand Up @@ -65,11 +65,13 @@ public static void main(String[] args) throws Exception {
testCpuSet((((numCpus - 1) / 2) + 1) + "-" + (numCpus - 1));
}
testCpuSet(IntStream.range(0, numCpus).mapToObj(a -> Integer.toString(a)).collect(Collectors.joining(",")));
testCpuSet("0", true /* additional cgroup fs mount */);

testCpuQuota(50 * 1000, 100 * 1000);
testCpuQuota(100 * 1000, 100 * 1000);
testCpuQuota(150 * 1000, 100 * 1000);
testCpuQuota(400 * 1000, 100 * 1000);
testCpuQuota(200 * 1000, 100 * 1000, true /* additional cgroup fs mount */);

testCpuShares(256);
testCpuShares(2048);
Expand Down Expand Up @@ -108,10 +110,18 @@ private static void testCpuSetMems(String value) throws Exception {
}

private static void testCpuSet(String value) throws Exception {
testCpuSet(value, false);
}

private static void testCpuSet(String value, boolean addCgroupMount) throws Exception {
Common.logNewTestCase("testCpuSet, value = " + value);
DockerRunOptions opts =
new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsCpuTester");
opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/");
if (addCgroupMount) {
// Extra cgroup mount should be ignored by product code
opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
}
opts.addJavaOpts("-cp", "/test-classes/");
opts.addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED");
opts.addClassOptions("cpusets", value);
Expand All @@ -120,10 +130,18 @@ private static void testCpuSet(String value) throws Exception {
}

private static void testCpuQuota(long quota, long period) throws Exception {
testCpuQuota(quota, period, false);
}

private static void testCpuQuota(long quota, long period, boolean addCgroupMount) throws Exception {
Common.logNewTestCase("testCpuQuota, quota = " + quota + ", period = " + period);
DockerRunOptions opts =
new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsCpuTester");
opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/");
if (addCgroupMount) {
// Extra cgroup mount should be ignored by product code
opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
}
opts.addDockerOpts("--cpu-period=" + period).addDockerOpts("--cpu-quota=" + quota);
opts.addJavaOpts("-cp", "/test-classes/").addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED");
opts.addClassOptions("cpuquota", quota + "", period + "");
Expand Down
10 changes: 10 additions & 0 deletions test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetrics.java
View file
Expand Up @@ -56,6 +56,8 @@ public static void main(String[] args) throws Exception {
try {
testMemoryLimit("200m");
testMemoryLimit("1g");
// Memory limit test with additional cgroup fs mounted
testMemoryLimit("500m", true /* cgroup fs mount */);

testMemoryAndSwapLimit("200m", "1g");
testMemoryAndSwapLimit("100m", "200m");
Expand Down Expand Up @@ -89,6 +91,10 @@ public static void main(String[] args) throws Exception {
}

private static void testMemoryLimit(String value) throws Exception {
testMemoryLimit(value, false);
}

private static void testMemoryLimit(String value, boolean addCgroupMount) throws Exception {
Common.logNewTestCase("testMemoryLimit, value = " + value);
DockerRunOptions opts =
new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsMemoryTester");
Expand All @@ -97,6 +103,10 @@ private static void testMemoryLimit(String value) throws Exception {
.addJavaOpts("-cp", "/test-classes/")
.addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED")
.addClassOptions("memory", value);
if (addCgroupMount) {
// Extra cgroup mount should be ignored by product code
opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
}
DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0).shouldContain("TEST PASSED!!!");
}

Expand Down

0 comments on commit 2a3eea0

Please sign in to comment.