> 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 
> proposed 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 incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains 89 additional commits 
since the last revision:

 - Merge branch 'jdk-8322420_cgroup_hierarchy_walk_init' into 
jdk-8336881-metrics-systemd-slice
 - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
 - 8339419: [s390x] Problemlist compiler/c2/irTests/TestIfMinMax.java
   
   Reviewed-by: thartmann
 - 8338916: Build warnings about overriding recipe for jvm-ldflags.txt
   
   Reviewed-by: jwaters, erikj
 - 8339336: Fix build system whitespace to adhere to coding conventions
   
   Reviewed-by: erikj
 - 8339214: Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc)
   
   Reviewed-by: asotona
 - 8338740: java/net/httpclient/HttpsTunnelAuthTest.java fails with 
java.io.IOException: HTTP/1.1 header parser received no bytes
   
   Reviewed-by: djelinski, jpai
 - 8325397: sun/java2d/Disposer/TestDisposerRace.java fails in linux-aarch64
   
   Reviewed-by: alanb
 - 8339401: Optimize ClassFile load and store instructions
   
   Reviewed-by: liach, redestad
 - 8339364: AIX build fails: various unused variable and function warnings
   
   Reviewed-by: mdoerr, clanger, jwaters
 - ... and 79 more: https://git.openjdk.org/jdk/compare/e4f2f439...7beccf23

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/20280/files
  - new: https://git.openjdk.org/jdk/pull/20280/files/2d918ca4..7beccf23

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=20280&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20280&range=03-04

  Stats: 10214 lines in 362 files changed: 6051 ins; 1713 del; 2450 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

Reply via email to