Re: RFR: 8226575: OperatingSystemMXBean should be made container aware
Hi Severin, On Thu, 2019-07-18 at 18:37 +0200, Severin Gehwolf wrote: > Hi Andrew, > > src/jdk.management/share/classes/module-info.java > > I don't think you need to explicitly require java.base. Everything > depends on java.base. Is there a specific reason why that was needed? > Ah, you're right that that's not needed. I figured java.base should be implicitly depended upon, but the first run through failed due to module visibility. The actual fix needed was the other module-info change in the patch, ie: +exports jdk.internal.platform to +jdk.management; in the java.base module-info. I've gone ahead and removed the unnecessary addition. > I've noticed that this test fails with your patch: > > FAILED: > com/sun/management/OperatingSystemMXBean/GetSystemCpuLoad.java > > Fails with: > java.lang.RuntimeException: getSystemCpuLoad() returns > 173995.48501979 which is not in the [0.0,1.0] interval > at GetSystemCpuLoad.main(GetSystemCpuLoad.java:43) What did you run to get this test to execute? I hadn't hit that failure in running `make test TEST=tier1`, but I can verify the failure if I run specifically that class, so I'm unsure what larger suite or tier it's included in. I'll need to go back and rethink how this system load calculation can be done. > > This makes me wonder what the effect of this patch is on Linux > *outside* a container. Have you verified that Metric values > correspond > to what whas previously returned via native methods? Could you > verify, > please and tell us the before/after values? Thanks! Other than the system load, the other memory-related values appear to be correct both on bare metal and in Docker. Manually verifying the tests in com/sun/management/OperatingSystemMXBean with the "trace" argument and the patch applied (system load change removed), the values printed match my host machine's /proc/meminfo. Running jtreg:test/hotspot/jtreg/containers/docker tests also shows that the Docker tests succeed and the reported values within a container are as expected as well. I can't see that there are any tests to verify the Metrics implementation itself. Am I just missing them or have none been written? > > I wonder if we should split the OperatingSystemMXBean tests into it's > own (and not piggy-back on > TestCPUAwareness.java/TestMemoryAwareness.java). Have you considered > that? > Sure, I could do that for the next review round. Thanks, -- Andrew Azores Software Engineer, OpenJDK Team Red Hat
RFR: 8226575: OperatingSystemMXBean should be made container aware
Hi all, Please review this change that addresses JDK-8226575 according to a previous discussion on this list [0][1]. The webrev has been kindly uploaded on my behalf by Severin Gehwolf, since I am not an author. The initial problem was that the com.sun.management.OperatingSystemMXBean was inconsistent about its awareness of applicable container limits. On the one hand, the (inherited) getAvailableProcessors() return value is consistent with the container-aware Runtime.availableProcessors(). But on the other hand, c.s.m.OperatingSystemMXBean's getTotalPhysicalMemorySize(), getFreePhysicalMemorySize(), getTotalSwapSpaceSize(), and getFreeSwapSpaceSize() returned values reflecting the physical host machine with no awareness of container limits. getSystemCpuLoad() has also been updated to use cgroup-accounted load calculation. The fix applied is to use the JDK internal Metrics API to determine container memory limits and CPU accounting and use these values instead, if available, otherwise falling back on the pre-existing native implementations. bug: https://bugs.openjdk.java.net/browse/JDK-8226575 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/aazores/JDK-8226575/01/webrev/ [0] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-June/028439.html [1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-July/028709.html Thanks, -- Andrew Azores Software Engineer, OpenJDK Team Red Hat
Re: OperatingSystemMXBean unaware of container memory limits
Hi Bob (and all), On Fri, 2019-06-21 at 08:50 -0400, Bob Vandette wrote: > > > Fixing the existing OS Mbean is pretty straight forward. Just have > each method call the new Metrics API, check for error returns > in case this API is not supported on this platform and if success, > return result, otherwise call existing logic. > I've had free time to circle back to this issue lately, so I've prepared a patch which modifies the OperatingSystemMXBean as you've described here. This seems to work perfectly well for getTotalPhysicalMemorySize, getFreePhysicalMemorySize, and getTotalSwapSpaceSize. getFreeSwapSpaceSize looks at a glance to be implementable by comparing Metrics' getMemoryAndSwapUsage, getMemoryUsage, and getMemoryAndSwap limit, but the implementation would only provide an approximation (and not necessarily accurately) since the amount of memory/swap used would likely change in between or due to subsequent calls to the Metrics API. Also, I'm not sure that there are reasonable equivalents for all of the other metrics implemented in jdk.management/unix/classes/com/sun/management/internal/OperatingSystem Impl.java available from Metrics or elsewhere which make sense in a containerized environment (ex. getCommittedVirtualMemorySize). So, before I proceed much further with this patch, does anyone have input on how these issues should be handled such that the OperatingSystemMXBean is self-consistent wrt its container awareness? -- Andrew Azores Software Engineer, OpenJDK Team Red Hat
OperatingSystemMXBean unaware of container memory limits
Hi all,
Apologies if this is not the most appropriate list, in which case
please direct me where to go.
I've noticed a surprising result from the
com.sun.management.OperatingSystemMXBean implementation when running in
a containerized (specifically, using Docker on Linux) environment. The
bean appears to be container-aware for processors, in that running with
Docker option `--cpus 1.0` for example, on a multicore system, will
cause both java.lang.Runtime#availableProcessors and
java.lang.management.OperatingSystemMXBean#getAvailableProcessors /
com.sun.management.OperatingSystemMXBean#getAvailableProcessors to
return 1. However, the Docker option `--memory 100M` (or any other
limit value) is not reflected in the value returned by
com.sun.management.OperatingSystemMXBeam#getTotalPhysicalMemorySize,
and instead the returned value is still the total physical memory of
the host machine - of which only a small portion may actually be
available to the "Operating System" of the JVM. Similarly for the
methods regarding free physical memory, total swap, and free swap.
I have attached a patch which adds a small reproducer to the existing
MemoryAwareness test.
This seems like a bug to me, since if the imposed container limit on
processors as a resource is included as part of the "Operating System"
resource reporting, then surely memory resources should be reported the
same way. As I said, I found the current behaviour quite surprising.
--
Andrew Azores
Software Engineer, OpenJDK Team
Red Hat
# HG changeset patch
# User Andrew Azores
# Date 1561037890 14400
# Thu Jun 20 09:38:10 2019 -0400
# Node ID 25d8a60c172afc57902e8655889dd47ef26b9ca2
# Parent 8d50ff464ae56ad62679b44afa705276c7aece50
[mq]: test
diff --git a/test/hotspot/jtreg/containers/docker/CheckOperatingSystemMXBean.java b/test/hotspot/jtreg/containers/docker/CheckOperatingSystemMXBean.java
new file mode 100644
--- /dev/null
+++ b/test/hotspot/jtreg/containers/docker/CheckOperatingSystemMXBean.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * 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.
+ */
+
+import com.sun.management.OperatingSystemMXBean;
+import java.lang.management.ManagementFactory;
+
+public class CheckOperatingSystemMXBean {
+
+public static void main(String[] args) {
+System.out.println("Checking OperatingSystemMXBean");
+
+OperatingSystemMXBean osBean = (OperatingSystemMXBean) ManagementFactory.getOperatingSystemMXBean();
+System.out.println(String.format("Runtime.availableProcessors: %d", Runtime.getRuntime().availableProcessors()));
+System.out.println(String.format("OperatingSystemMXBean.getAvailableProcessors: %d", osBean.getAvailableProcessors()));
+System.out.println(String.format("OperatingSystemMXBean.getTotalPhysicalMemorySize: %d", osBean.getTotalPhysicalMemorySize()));
+System.out.println(String.format("OperatingSystemMXBean.getTotalSwapSpaceSize: %d", osBean.getTotalSwapSpaceSize()));
+}
+
+}
diff --git a/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java b/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
--- a/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
+++ b/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
@@ -30,7 +30,7 @@
* @modules java.base/jdk.internal.misc
* java.management
* jdk.jartool/sun.tools.jar
- * @build AttemptOOM sun.hotspot.WhiteBox PrintContainerInfo
+ * @build AttemptOOM sun.hotspot.WhiteBox PrintContainerInfo CheckOperatingSystemMXBean
* @run driver ClassFileInstaller -jar whitebox.jar sun.hotspot.WhiteBox sun.hotspot.WhiteBox$WhiteBoxPermission
* @run driver TestMemoryAwareness
*/
@@ -62,8 +62,25 @@
// Add extra 10 Mb to allocator limit, to be sure to cause OOM
testOOM("256m", 256 + 10);
+testMXBeanAwareness(
+"1.0", "
