You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by gi...@apache.org on 2022/04/27 18:18:50 UTC
[druid] branch master updated: JvmMonitor: Handle more generation and collector scenarios. (#12469)
This is an automated email from the ASF dual-hosted git repository.
gian pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 72d15ab321 JvmMonitor: Handle more generation and collector scenarios. (#12469)
72d15ab321 is described below
commit 72d15ab321851e8b97ac10fcd6c71c5828af3ab6
Author: Gian Merlino <gi...@imply.io>
AuthorDate: Wed Apr 27 11:18:40 2022 -0700
JvmMonitor: Handle more generation and collector scenarios. (#12469)
* JvmMonitor: Handle more generation and collector scenarios.
ZGC on Java 11 only has a generation 1 (there is no 0). This causes
a NullPointerException when trying to extract the spacesCount for
generation 0. In addition, ZGC on Java 15 has a collector number 2
but no spaces in generation 2, which breaks the assumption that
collectors always have same-numbered spaces.
This patch adjusts things to be more robust, enabling the JvmMonitor
to work properly for ZGC on both Java 11 and 15.
* Test adjustments.
* Improve surefire arglines.
* Need a placeholder
---
benchmarks/pom.xml | 5 +-
core/pom.xml | 1 +
.../apache/druid/java/util/metrics/JvmMonitor.java | 60 ++++++++++++++++++----
.../druid/java/util/metrics/JvmMonitorTest.java | 37 +++++++++++++
pom.xml | 12 ++++-
processing/pom.xml | 6 ++-
6 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml
index c20555e4a8..bd5730ea7d 100644
--- a/benchmarks/pom.xml
+++ b/benchmarks/pom.xml
@@ -272,7 +272,10 @@
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
- <argLine>@{jacocoArgLine}</argLine>
+ <argLine>
+ @{jacocoArgLine}
+ ${jdk.surefire.argLine}
+ </argLine>
</configuration>
</plugin>
</plugins>
diff --git a/core/pom.xml b/core/pom.xml
index bfaa77be03..0b12558e24 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -432,6 +432,7 @@
<useManifestOnlyJar>false</useManifestOnlyJar>
<argLine>
@{jacocoArgLine}
+ ${jdk.surefire.argLine}
-Djava.library.path=${project.build.directory}/hyperic-sigar-${sigar.base.version}/sigar-bin/lib/
-Duser.language=en
</argLine>
diff --git a/core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java b/core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java
index c36d0f92b2..77f4033a3d 100644
--- a/core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java
+++ b/core/src/main/java/org/apache/druid/java/util/metrics/JvmMonitor.java
@@ -22,6 +22,8 @@ package org.apache.druid.java.util.metrics;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import it.unimi.dsi.fastutil.ints.IntRBTreeSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
@@ -40,10 +42,15 @@ import java.lang.management.MemoryUsage;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
public class JvmMonitor extends FeedDefiningMonitor
{
private static final Logger log = new Logger(JvmMonitor.class);
+ private static final Pattern PATTERN_GC_GENERATION =
+ Pattern.compile("^sun\\.gc\\.(?:generation|collector)\\.(\\d+)\\..*");
private final Map<String, String[]> dimensions;
private final long pid;
@@ -170,12 +177,43 @@ public class JvmMonitor extends FeedDefiningMonitor
// in JDK11 jdk.internal.perf.Perf is not accessible, unless
// --add-exports java.base/jdk.internal.perf=ALL-UNNAMED is set
log.warn("Cannot initialize GC counters. If running JDK11 and above,"
- + " add --add-exports java.base/jdk.internal.perf=ALL-UNNAMED"
+ + " add --add-exports=java.base/jdk.internal.perf=ALL-UNNAMED"
+ " to the JVM arguments to enable GC counters.");
}
return null;
}
+ @VisibleForTesting
+ static IntSet getGcGenerations(final Set<String> jStatCounterNames)
+ {
+ final IntSet retVal = new IntRBTreeSet();
+
+ for (final String counterName : jStatCounterNames) {
+ final Matcher m = PATTERN_GC_GENERATION.matcher(counterName);
+ if (m.matches()) {
+ retVal.add(Integer.parseInt(m.group(1)));
+ }
+ }
+
+ return retVal;
+ }
+
+ @VisibleForTesting
+ static String getGcGenerationName(final int genIndex)
+ {
+ switch (genIndex) {
+ case 0:
+ return "young";
+ case 1:
+ return "old";
+ case 2:
+ // Removed in Java 8 but still actual for previous Java versions
+ return "perm";
+ default:
+ return String.valueOf(genIndex);
+ }
+ }
+
/**
* The following GC-related code is partially based on
* https://github.com/aragozin/jvm-tools/blob/e0e37692648951440aa1a4ea5046261cb360df70/
@@ -191,11 +229,8 @@ public class JvmMonitor extends FeedDefiningMonitor
final JStatData jStatData = JStatData.connect(pid);
final Map<String, JStatData.Counter<?>> jStatCounters = jStatData.getAllCounters();
- generations.add(new GcGeneration(jStatCounters, 0, "young"));
- generations.add(new GcGeneration(jStatCounters, 1, "old"));
- // Removed in Java 8 but still actual for previous Java versions
- if (jStatCounters.containsKey("sun.gc.generation.2.name")) {
- generations.add(new GcGeneration(jStatCounters, 2, "perm"));
+ for (int genIndex : getGcGenerations(jStatCounters.keySet())) {
+ generations.add(new GcGeneration(jStatCounters, genIndex, getGcGenerationName(genIndex)));
}
}
@@ -210,6 +245,7 @@ public class JvmMonitor extends FeedDefiningMonitor
private class GcGeneration
{
private final String name;
+ @Nullable
private final GcGenerationCollector collector;
private final List<GcGenerationSpace> spaces = new ArrayList<>();
@@ -217,11 +253,13 @@ public class JvmMonitor extends FeedDefiningMonitor
{
this.name = StringUtils.toLowerCase(name);
- long spacesCount = ((JStatData.LongCounter) jStatCounters.get(
- StringUtils.format("sun.gc.generation.%d.spaces", genIndex)
- )).getLong();
- for (long spaceIndex = 0; spaceIndex < spacesCount; spaceIndex++) {
- spaces.add(new GcGenerationSpace(jStatCounters, genIndex, spaceIndex));
+ final String spacesCountKey = StringUtils.format("sun.gc.generation.%d.spaces", genIndex);
+
+ if (jStatCounters.containsKey(spacesCountKey)) {
+ final long spacesCount = ((JStatData.LongCounter) jStatCounters.get(spacesCountKey)).getLong();
+ for (long spaceIndex = 0; spaceIndex < spacesCount; spaceIndex++) {
+ spaces.add(new GcGenerationSpace(jStatCounters, genIndex, spaceIndex));
+ }
}
if (jStatCounters.containsKey(StringUtils.format("sun.gc.collector.%d.name", genIndex))) {
diff --git a/core/src/test/java/org/apache/druid/java/util/metrics/JvmMonitorTest.java b/core/src/test/java/org/apache/druid/java/util/metrics/JvmMonitorTest.java
index 1e5cff5016..d5a2890e08 100644
--- a/core/src/test/java/org/apache/druid/java/util/metrics/JvmMonitorTest.java
+++ b/core/src/test/java/org/apache/druid/java/util/metrics/JvmMonitorTest.java
@@ -19,6 +19,8 @@
package org.apache.druid.java.util.metrics;
+import com.google.common.collect.ImmutableSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
import org.apache.druid.java.util.emitter.core.Emitter;
import org.apache.druid.java.util.emitter.core.Event;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
@@ -56,6 +58,41 @@ public class JvmMonitorTest
}
}
+ @Test
+ public void testGetGcGenerations()
+ {
+ Assert.assertEquals(
+ IntSet.of(0, 1),
+ JvmMonitor.getGcGenerations(
+ ImmutableSet.of(
+ "sun.gc.collector.0.name",
+ "sun.gc.collector.1.name",
+ "sun.gc.generation.1.spaces"
+ )
+ )
+ );
+
+ Assert.assertEquals(
+ IntSet.of(1, 2),
+ JvmMonitor.getGcGenerations(
+ ImmutableSet.of(
+ "sun.gc.generation.1.spaces",
+ "sun.gc.collector.2.name",
+ "sun.gc.somethingelse.3.name"
+ )
+ )
+ );
+ }
+
+ @Test
+ public void testGetGcGenerationName()
+ {
+ Assert.assertEquals("young", JvmMonitor.getGcGenerationName(0));
+ Assert.assertEquals("old", JvmMonitor.getGcGenerationName(1));
+ Assert.assertEquals("perm", JvmMonitor.getGcGenerationName(2));
+ Assert.assertEquals("3", JvmMonitor.getGcGenerationName(3));
+ }
+
private static class GcTrackingEmitter implements Emitter
{
private Number oldGcCount;
diff --git a/pom.xml b/pom.xml
index ad68b7f533..9bf6e89acb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -120,6 +120,7 @@
<com.google.apis.client.version>1.26.0</com.google.apis.client.version>
<com.google.apis.compute.version>v1-rev20190607-${com.google.apis.client.version}</com.google.apis.compute.version>
<com.google.apis.storage.version>v1-rev20190523-${com.google.apis.client.version}</com.google.apis.storage.version>
+ <jdk.surefire.argLine><!-- empty placeholder --></jdk.surefire.argLine>
<repoOrgId>apache.snapshots</repoOrgId>
<repoOrgName>Apache Snapshot Repository</repoOrgName>
<repoOrgUrl>https://repository.apache.org/snapshots</repoOrgUrl>
@@ -1635,6 +1636,13 @@
<activation>
<jdk>[9,)</jdk>
</activation>
+ <properties>
+ <jdk.surefire.argLine>
+ <!-- required for JvmMonitor tests on Java 11+ -->
+ --add-exports=java.base/jdk.internal.perf=ALL-UNNAMED
+ --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED
+ </jdk.surefire.argLine>
+ </properties>
<build>
<plugins>
<plugin>
@@ -1718,7 +1726,9 @@
<trimStackTrace>false</trimStackTrace>
<!-- locale settings must be set on the command line before startup -->
<!-- set heap size to work around https://github.com/travis-ci/travis-ci/issues/3396 -->
- <argLine>-Xmx768m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8
+ <argLine>
+ ${jdk.surefire.argLine}
+ -Xmx768m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8
-Duser.timezone=UTC -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager
-Daws.region=us-east-1 <!-- required for s3-related unit tests -->
<!--@TODO After fixing https://github.com/apache/druid/issues/4964 remove this parameter-->
diff --git a/processing/pom.xml b/processing/pom.xml
index c8cdef020d..fedf9a113e 100644
--- a/processing/pom.xml
+++ b/processing/pom.xml
@@ -284,6 +284,7 @@
<!-- set default options -->
<argLine>
@{jacocoArgLine}
+ ${jdk.surefire.argLine}
-Xmx512m
-XX:MaxDirectMemorySize=2500m
-Duser.language=en
@@ -310,7 +311,10 @@
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
- <argLine>-server -Xms3G -Xmx3G -Djub.consumers=CONSOLE,H2 -Djub.db.file=benchmarks/benchmarks</argLine>
+ <argLine>
+ ${jdk.surefire.argLine}
+ -server -Xms3G -Xmx3G -Djub.consumers=CONSOLE,H2 -Djub.db.file=benchmarks/benchmarks
+ </argLine>
<groups>org.apache.druid.collections.test.annotation.Benchmark</groups>
<excludedGroups>org.apache.druid.collections.test.annotation.Dummy</excludedGroups>
</configuration>
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org