Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8293540: [Metrics] Incorrectly detected resource limits with additional cgroup fs mounts #10248

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -318,30 +318,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 @@ -355,10 +336,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 @@ -367,6 +345,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
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 @@ -170,6 +175,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 @@ -181,6 +192,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
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
@@ -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
jerboaa marked this conversation as resolved.
Show resolved Hide resolved
* @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");
jerboaa marked this conversation as resolved.
Show resolved Hide resolved
}
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
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
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 @@ -85,6 +87,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 @@ -93,6 +99,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