8254001: [Metrics] Enhance parsing of cgroup interface files for version detection #1020

andrewazores · 2022-04-07T15:34:54Z

Backport commit a50725db2ab621e1a17cb5505f78e4bc73972a27 for JDK-8254001, enhancement to cgroups interface parsing and version detection.

The patch applied mostly cleanly with only two minor conflicts in CgroupV1Subsystem.java. These conflicts were simply hunks that failed to apply because the surrounding context did not match due to later changes in the internal implementation that are not present in JDK11. Since the patch being backported wholly replaces or removes these implementations anyway, my resolution was to simply remove the old implementation and accept the new implementation from the backport patch.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8254001: [Metrics] Enhance parsing of cgroup interface files for version detection

Reviewers

Severin Gehwolf (@jerboaa - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk11u-dev pull/1020/head:pull/1020
$ git checkout pull/1020

Update a local copy of the PR:
$ git checkout pull/1020
$ git pull https://git.openjdk.java.net/jdk11u-dev pull/1020/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1020

View PR using the GUI difftool:
$ git pr show -t 1020

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk11u-dev/pull/1020.diff

Backport commit a50725d for JDK-8254001, enhancement to cgroups interface parsing and version detection.

bridgekeeper · 2022-04-07T15:35:41Z

Hi @andrewazores, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user andrewazores" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

openjdk · 2022-04-07T15:36:39Z

This backport pull request has now been updated with issue from the original commit.

andrewazores · 2022-04-07T15:43:11Z

/covered

bridgekeeper · 2022-04-07T15:44:31Z

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

jerboaa · 2022-04-12T09:54:11Z

@robilad Hi! Could you please help with OCA verification for Andrew? He works for Red Hat and is covered by Red Hat's OCA. Thanks!

jerboaa

Once change required due to an out-of-order backport to 11u.

jerboaa · 2022-04-12T10:19:49Z

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java

            }
+            selfCgroupLines.map(line -> line.split(":"))


This will have to also include the fix from JDK-8272124 so that not regress. See: openjdk/jdk@4d6593c

It'll be covered by TestCgroupSubsystemFactory once JDK-8284102, a 11u-only bug, gets fixed.

Okay, so rather than backporting the whole 8272124 I should just cherry-pick out that single fix to this file for inclusion in this PR?

Yes. JDK-8272124 is in 11u since 11.0.13. The backported fix was adapted and got adapted again with the initial cgroups v2 support patch in java.base. Basically we need to keep the code handling extra colons :) Before this patch it was done here:
https://github.com/openjdk/jdk11u-dev/pull/1020/files#diff-463442e0351f5129ca93998b61b6ab5edb7f95e71092069e44e516d9180ea160L111-L117

Partial backport of commit 4d6593ce0243457e7431a5990957a8f880e0a3fb, fixing an NPE that can occur in cgroupv1 initialization when cgroup path contains a colon. Related test is not included since it will be covered later by JDK-8284102

jerboaa

LGTM. Hopefully we'll get the OCA verification resolved soon.

jerboaa · 2022-04-22T09:51:51Z

@robilad Gentle ping?

openjdk · 2022-04-27T23:01:06Z

@andrewazores This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8254001: [Metrics] Enhance parsing of cgroup interface files for version detection

Reviewed-by: sgehwolf

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 75 new commits pushed to the master branch:

234724c: 8284458: CodeHeapState::aggregate() leaks blob_name
73aea92: 8277396: [TESTBUG] In DefaultButtonModelCrashTest.java, frame is accessed from main thread
6480023: 8272167: AbsPathsInImage.java should skip *.dSYM directories
52aa769: 8269772: [macos-aarch64] test compilation failed with "SocketException: No buffer space available"
d279487: 8266318: Switch to macos prefix for macOS bundles
50e83c5: 8242538: java/security/SecureRandom/ThreadSafe.java failed on windows
e11476c: 8233551: [TESTBUG] SelectEditTableCell.java fails on MacOS
d14462f: 8234382: Test tools/javac/processing/model/testgetallmembers/Main.java using too small heap
43655b7: 8279520: SPNEGO has not passed channel binding info into the underlying mechanism
580a991: 8213440: Lingering INCLUDE_ALL_GCS in test_oopStorage_parperf.cpp
... and 65 more: https://git.openjdk.java.net/jdk11u-dev/compare/061c08348a4babe66ce1d5481b6c5d34e752c4cc...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@jerboaa) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mlbridge · 2022-04-27T23:05:03Z

Webrevs

00: Full (4849deea)

jerboaa · 2022-04-28T08:35:42Z

@andrewazores OCA is now approved. Please request for approval in the bug. See step 6 in: https://wiki.openjdk.java.net/display/JDKUpdates/How+to+contribute+a+fix

GoeLin · 2022-04-29T06:45:26Z

Hi, can you please enable the GHA actions and Pre-submit tests for this repo so they run next time? Thanks

andrewazores · 2022-04-29T14:47:09Z

Hi @GoeLin , I have enabled Actions on my fork. Is there a way to trigger the tests to run now?

andrewazores · 2022-04-29T15:31:33Z

/integrate

openjdk · 2022-04-29T15:33:16Z

@andrewazores
Your change (at version 4849dee) is now ready to be sponsored by a Committer.

jerboaa · 2022-04-29T15:41:15Z

/sponsor

openjdk · 2022-04-29T15:42:59Z

Going to push as commit 9c10f3e.
Since your change was applied there have been 75 commits pushed to the master branch:

234724c: 8284458: CodeHeapState::aggregate() leaks blob_name
73aea92: 8277396: [TESTBUG] In DefaultButtonModelCrashTest.java, frame is accessed from main thread
6480023: 8272167: AbsPathsInImage.java should skip *.dSYM directories
52aa769: 8269772: [macos-aarch64] test compilation failed with "SocketException: No buffer space available"
d279487: 8266318: Switch to macos prefix for macOS bundles
50e83c5: 8242538: java/security/SecureRandom/ThreadSafe.java failed on windows
e11476c: 8233551: [TESTBUG] SelectEditTableCell.java fails on MacOS
d14462f: 8234382: Test tools/javac/processing/model/testgetallmembers/Main.java using too small heap
43655b7: 8279520: SPNEGO has not passed channel binding info into the underlying mechanism
580a991: 8213440: Lingering INCLUDE_ALL_GCS in test_oopStorage_parperf.cpp
... and 65 more: https://git.openjdk.java.net/jdk11u-dev/compare/061c08348a4babe66ce1d5481b6c5d34e752c4cc...master

Your commit was automatically rebased without conflicts.

jerboaa · 2022-04-29T15:43:04Z

Is there a way to trigger the tests to run now?

Force pushing to the branch. It's OK though. We've tested it as part of #1056 (a dependent PR which includes your patch).

openjdk · 2022-04-29T15:43:24Z

@jerboaa @andrewazores Pushed as commit 9c10f3e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Backport a50725d

10a02d5

Backport commit a50725d for JDK-8254001, enhancement to cgroups interface parsing and version detection.

bridgekeeper bot added the oca Needs verification of OCA signatory status label Apr 7, 2022

openjdk bot changed the title ~~Backport a50725db2ab621e1a17cb5505f78e4bc73972a27~~ 8254001: [Metrics] Enhance parsing of cgroup interface files for version detection Apr 7, 2022

openjdk bot added the backport label Apr 7, 2022

bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label Apr 7, 2022

jerboaa suggested changes Apr 12, 2022

View reviewed changes

jerboaa approved these changes Apr 15, 2022

View reviewed changes

bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels Apr 27, 2022

openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 27, 2022

openjdk bot mentioned this pull request Apr 28, 2022

8284102: [TESTBUG] [11u] Retroactively add regression test for JDK-8272124 #1056

Closed

3 tasks

jmtd mentioned this pull request Apr 29, 2022

8262379: Add regression test for JDK-8257746 #1066

Closed

2 tasks

openjdk bot added the sponsor Pull request is ready to be sponsored label Apr 29, 2022

openjdk bot added the integrated Pull request has been integrated label Apr 29, 2022

openjdk bot closed this Apr 29, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Apr 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8254001: [Metrics] Enhance parsing of cgroup interface files for version detection #1020

8254001: [Metrics] Enhance parsing of cgroup interface files for version detection #1020

andrewazores commented Apr 7, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 7, 2022

openjdk bot commented Apr 7, 2022

andrewazores commented Apr 7, 2022

bridgekeeper bot commented Apr 7, 2022

jerboaa commented Apr 12, 2022

jerboaa left a comment

jerboaa Apr 12, 2022

andrewazores Apr 13, 2022

jerboaa Apr 13, 2022

jerboaa left a comment

jerboaa commented Apr 22, 2022

openjdk bot commented Apr 27, 2022 •

edited

Loading

mlbridge bot commented Apr 27, 2022

jerboaa commented Apr 28, 2022

GoeLin commented Apr 29, 2022

andrewazores commented Apr 29, 2022

andrewazores commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

jerboaa commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

jerboaa commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

8254001: [Metrics] Enhance parsing of cgroup interface files for version detection #1020

8254001: [Metrics] Enhance parsing of cgroup interface files for version detection #1020

Conversation

andrewazores commented Apr 7, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Apr 7, 2022

openjdk bot commented Apr 7, 2022

andrewazores commented Apr 7, 2022

bridgekeeper bot commented Apr 7, 2022

jerboaa commented Apr 12, 2022

jerboaa left a comment

Choose a reason for hiding this comment

jerboaa Apr 12, 2022

Choose a reason for hiding this comment

andrewazores Apr 13, 2022

Choose a reason for hiding this comment

jerboaa Apr 13, 2022

Choose a reason for hiding this comment

jerboaa left a comment

Choose a reason for hiding this comment

jerboaa commented Apr 22, 2022

openjdk bot commented Apr 27, 2022 • edited Loading

mlbridge bot commented Apr 27, 2022

Webrevs

jerboaa commented Apr 28, 2022

GoeLin commented Apr 29, 2022

andrewazores commented Apr 29, 2022

andrewazores commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

jerboaa commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

jerboaa commented Apr 29, 2022

openjdk bot commented Apr 29, 2022

andrewazores commented Apr 7, 2022 •

edited by openjdk bot

Loading

openjdk bot commented Apr 27, 2022 •

edited

Loading