Add accessors for container CPU shares #150
Conversation
Generate changelog in
|
jvm-diagnostics/src/main/java/com/palantir/jvm/diagnostics/CpuSharesAccessor.java
Outdated
Show resolved
Hide resolved
| * Returns the relative-weighted CPU shares detected by the JVM. In k8s, this should be a millicore value. | ||
| * Returns {@link OptionalLong#empty()} if information is unavailable (and this accessor is not returned itself | ||
| * if cpu shares are not supported). | ||
| */ | ||
| OptionalLong getCpuShares(); |
There was a problem hiding this comment.
Might be worth adding to comments that https://bugs.openjdk.org/browse/JDK-8281181 / https://bugs.openjdk.org/browse/JDK-8281571 / https://bugs.openjdk.org/browse/JDK-8279484 / openjdk/jdk@e07fd39 immediately deprecates UseContainerCpuShares in JDK 19, 11.0.17, and 17.0.5
There was a problem hiding this comment.
May be also worth mentioning https://danluu.com/cgroup-throttling/
| private final jdk.internal.platform.Metrics metrics = jdk.internal.platform.Metrics.systemMetrics(); | ||
|
|
||
| boolean isEnabled() { | ||
| return metrics.getCpuShares() != -2; |
There was a problem hiding this comment.
From https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/platform/Metrics.java#L153-L169 I think we might want to adjust this to:
shares value, -1 if the metric is not available or
* -2 if cpu shares are not supported.
| return metrics.getCpuShares() != -2; | |
| return metrics.getCpuShares() >= 0; |
There was a problem hiding this comment.
I wanted to distinguish between "I know what cpu shares are, but I'm not using them" from "I'm not sure how to tell if cpu shares are in play". In the former case, we return an accessor that provides OptionalLong.emty, and in the latter case we return an empty Optional.
There was a problem hiding this comment.
Would you prefer to avoid that distinction?
There was a problem hiding this comment.
I assume you'll expose this as a gauge metric? Would you consume it as something like this?
JvmDiagnostics.cpuShares().ifPresent(shares -> registerGauge(() -> shares.getAsLong()))
There was a problem hiding this comment.
Both a gauge metric, and our startup logging (the latter is where we can distinguish between unavailable and unsupported)
| * Returns an {@link HotspotCpuSharesAccessor}. This functionality is not supported on all java runtimes, | ||
| * and an {@link Optional#empty()} is returned in cases cpu share information is not supported. | ||
| */ | ||
| public static Optional<CpuSharesAccessor> cpuShares() { |
There was a problem hiding this comment.
I don't have a great way to do so with the tools we have in this repo, unless we match the environment circle uses -- unfortunately tests won't work locally in that case
There was a problem hiding this comment.
I'm curious what circle returns on say OpenJDK 17.0.5 vs 17.0.4. We could add some JUnit assumptions around JDK version, OS, and compare to Runtime number of processors.
There was a problem hiding this comment.
I think this value will return the same results, but processor count may not (certainly not in our metrics)
|
Released 0.3.0 |
This is difficult to test in a reasonable way.