> Please review this fix for cgroups-based metrics reporting in the > `jdk.internal.platform` package. This fix is supposed to address wrong > reporting of certain limits if the limits aren't set at the leaf nodes. > > For example, on cg v2, the memory limit interface file is `memory.max`. > Consider a cgroup path of `/a/b/c/d`. The current code only reports the > limits (via Metrics) correctly if it's set at `/a/b/c/d/memory.max`. However, > some systems - like a systemd slice - sets those limits further up the > hierarchy. For example at `/a/b/c/memory.max`. `/a/b/c/d/memory.max` might be > set to the value `max` (for unlimited), yet `/a/b/c/memory.max` would report > the actual limit value (e.g. `1048576000`). > > This patch addresses this issue by: > > 1. Refactoring the interface lookup code to relevant controllers for > cpu/memory. The CgroupSubsystem classes then delegate to those for the > lookup. This facilitates having an API for the lookup of an updated limit in > step 2. > 2. Walking the full hierarchy of the cgroup path (if any), looking for a > lower limit than at the leaf. Note that it's not possible to raise the limit > set at a path closer to the root via the interface file at a > further-to-the-leaf-level. The odd case out seems to be `max` values on some > systems (which seems to be the default value). > > As an optimization this hierarchy walk is skipped on containerized systems > (like K8S), where the limits are set in interface files at the leaf nodes of > the hierarchy. Therefore there should be no change on those systems. > > This patch depends on the Hotspot change implementing the same for the JVM so > that `Metrics.isContainerized()` works correctly on affected systems where > `-XshowSettings:system` currently reports `System not containerized` due to > the missing JVM fix. A test framework for such hierarchical systems has been > added in [JDK-8333446](https://bugs.openjdk.org/browse/JDK-8333446). This > patch adds a test using that framework among some simpler unit tests. > > Thoughts? > > Testing: > > - [x] GHA > - [x] Container tests on Linux x86_64 on cg v1 and cg v2 systems > - [x] Some manual testing using systemd slices
Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 35 commits: - Merge branch 'master' into jdk-8336881-metrics-systemd-slice - Add exclusive access dirs directive for systemd tests - Merge branch 'master' into jdk-8336881-metrics-systemd-slice - Merge branch 'master' into jdk-8336881-metrics-systemd-slice - Improve systemd slice test for non-root on cg v2 - Fix unit tests - Add JVM_HostActiveProcessorCount using JVM api - Merge branch 'jdk-8322420_cgroup_hierarchy_walk_init' into jdk-8336881-metrics-systemd-slice - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init - Merge branch 'jdk-8322420_cgroup_hierarchy_walk_init' into jdk-8336881-metrics-systemd-slice - ... and 25 more: https://git.openjdk.org/jdk/compare/521effe0...03699b08 ------------- Changes: https://git.openjdk.org/jdk/pull/20280/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20280&range=09 Stats: 1595 lines in 27 files changed: 1344 ins; 152 del; 99 mod Patch: https://git.openjdk.org/jdk/pull/20280.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20280/head:pull/20280 PR: https://git.openjdk.org/jdk/pull/20280