JDK-8221340 : [TESTBUG] TestCgroupMetrics.java fails after fix for JDK-8219562
  • Type: Bug
  • Component: core-libs
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-03-22
  • Updated: 2020-11-30
  • Resolved: 2019-05-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 13 Other
11.0.5Fixed 13 b21Fixed openjdk8u282Fixed
Related Reports
Relates :  
JDK-8220579 - [Containers] SubSystem.java out of sync with osContainer_linux.cpp
Relates :  
JDK-8219562 - Line of code in osContainer_linux.cpp#L102 appears unreachable
Description
Change were made in JDK-8219562 to osContainer_linux.cpp to correct the parsing of of the /proc/self/mountinfo file but corresponding changes to the Metrics API and Container tests need to be done to match this change.

Here is a proposed fix:

diff --git a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
--- a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
+++ b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
@@ -60,7 +60,7 @@
                     path = mountPoint;
                 }
                 else {
-                    if (root.indexOf(cgroupPath) == 0) {
+                    if (cgroupPath.indexOf(root) == 0) {
                         if (cgroupPath.length() > root.length()) {
                             String cgroupSubstr = cgroupPath.substring(root.length());
                             path = mountPoint + cgroupSubstr;
diff --git a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
--- a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
+++ b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
@@ -85,7 +85,7 @@
                 String mountPoint = paths[1];
                 if (root != null && cgroupPath != null) {
                     if (root.equals("/")) {
-                        if (cgroupPath.equals("/")) {
+                        if (!cgroupPath.equals("/")) {
                             finalPath = mountPoint + cgroupPath;
                         } else {
                             finalPath = mountPoint;
@@ -94,7 +94,7 @@
                         if (root.equals(cgroupPath)) {
                             finalPath = mountPoint;
                         } else {
-                            if (root.indexOf(cgroupPath) == 0) {
+                            if (cgroupPath.indexOf(root) == 0) {
                                 if (cgroupPath.length() > root.length()) {
                                     String cgroupSubstr = cgroupPath.substring(root.length());
                                     finalPath = mountPoint + cgroupSubstr;
@@ -103,7 +103,7 @@
                         }
                     }
                 }
-                subSystemPaths.put(subSystem, new String[]{finalPath});
+                subSystemPaths.put(subSystem, new String[]{finalPath, mountPoint});
             }
         }
     }
Comments
Fix Request (OpenJDK 8u): The intention is to get JDK-8220579 approved for OpenJDK 8u. That introduces a test failure, because MetricsTester.java duplicates some of the code in the JDK in the test libraries. The sync mix-up got fixed with this patch. The proposal is to get JDK-8219562 and this bug backported too. This bug so as to fix the test failure caused by JDK-8220579 and JDK-8219562 so as to keep hotspot/jdk code in sync. This is a low risk patch and it applies clean (modulo path unshuffeling). Container tests pass with this. We have seen no issues with those in 11u.
26-11-2020
[~clanger] My understanding is that this fix is needed if either JDK-8219562 *or* JDK-8220579 gets backported. In my case it's the hunk above I've referred to for a test fix after JDK-8220579. Other hunks are test fixes for JDK-8219562. Since JDK-8219562 isn't yet backported the test still succeeds since it's more lenient after this fix, but still covering the relevant cases. To be sure I've updated to revision 2786541e4f91 (up to including JDK-8220579) in the jdk head tree and the test fails there too. So to some extent the description of this bug is wrong. Mixes concerns. TLDR; I'll backport JDK-8219562 as well, but it isn't strictly needed for test failures. It's more a matter of keeping code in sync between the JDK and hotspot. It's a straight forward backport and it makes sense for these changes to go in together.
07-08-2019
Hi Severin, what about JDK-8219562? Does this need to be backported as well? I see the title of this item refers to it.
07-08-2019
Fix Request (OpenJDK 11u): Patch for JDK-8220579 got approved for OpenJDK 11u. That introduces a test failure, because MetricsTester.java duplicates some of the code in the JDK in the test libraries. The sync mix-up got fixed with this patch in jdk/jdk. For that reason, I'd like to request backport of the test bug as well.
05-08-2019
This hunk: diff --git a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java --- a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java +++ b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java @@ -85,7 +85,7 @@ String mountPoint = paths[1]; if (root != null && cgroupPath != null) { if (root.equals("/")) { - if (cgroupPath.equals("/")) { + if (!cgroupPath.equals("/")) { ... is actually needed to synchronize code changes done with JDK-8220579. Adding a link.
05-08-2019