From 72d15ab321851e8b97ac10fcd6c71c5828af3ab6 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 27 Apr 2022 11:18:40 -0700 Subject: [PATCH] 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 + .../druid/java/util/metrics/JvmMonitor.java | 60 +++++++++++++++---- .../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 c20555e4a82..bd5730ea7d8 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -272,7 +272,10 @@ maven-surefire-plugin - @{jacocoArgLine} + + @{jacocoArgLine} + ${jdk.surefire.argLine} + diff --git a/core/pom.xml b/core/pom.xml index bfaa77be036..0b12558e24d 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -432,6 +432,7 @@ false @{jacocoArgLine} + ${jdk.surefire.argLine} -Djava.library.path=${project.build.directory}/hyperic-sigar-${sigar.base.version}/sigar-bin/lib/ -Duser.language=en 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 c36d0f92b21..77f4033a3da 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 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 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> 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 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 1e5cff5016b..d5a2890e089 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 ad68b7f5331..9bf6e89acb8 100644 --- a/pom.xml +++ b/pom.xml @@ -120,6 +120,7 @@ 1.26.0 v1-rev20190607-${com.google.apis.client.version} v1-rev20190523-${com.google.apis.client.version} + apache.snapshots Apache Snapshot Repository https://repository.apache.org/snapshots @@ -1635,6 +1636,13 @@ [9,) + + + + --add-exports=java.base/jdk.internal.perf=ALL-UNNAMED + --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED + + @@ -1718,7 +1726,9 @@ false - -Xmx768m -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 + + ${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 diff --git a/processing/pom.xml b/processing/pom.xml index c8cdef020d1..fedf9a113e9 100644 --- a/processing/pom.xml +++ b/processing/pom.xml @@ -284,6 +284,7 @@ @{jacocoArgLine} + ${jdk.surefire.argLine} -Xmx512m -XX:MaxDirectMemorySize=2500m -Duser.language=en @@ -310,7 +311,10 @@ maven-surefire-plugin - -server -Xms3G -Xmx3G -Djub.consumers=CONSOLE,H2 -Djub.db.file=benchmarks/benchmarks + + ${jdk.surefire.argLine} + -server -Xms3G -Xmx3G -Djub.consumers=CONSOLE,H2 -Djub.db.file=benchmarks/benchmarks + org.apache.druid.collections.test.annotation.Benchmark org.apache.druid.collections.test.annotation.Dummy