RATIS-1705. Fix metrics leak (#744)

apache · Aug 3, 2023 · 95b51e5 · 95b51e5
1 parent 9039959
commit 95b51e5
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 8 deletions.
diff --git a/ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java b/ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java
@@ -61,6 +61,14 @@ private void inc(String metricNamePrefix, Type t) {
     types.get(t)
         .computeIfAbsent(metricNamePrefix, prefix -> getRegistry().counter(prefix + t.getSuffix()))
         .inc();
+    final Map<String, LongCounter> counters = types.get(t);
+    LongCounter c = counters.get(metricNamePrefix);
+    if (c == null) {
+      synchronized (counters) {
+        c = counters.computeIfAbsent(metricNamePrefix, prefix -> getRegistry().counter(prefix + t.getSuffix()));
+      }
+    }
+    c.inc();
   }
 
   /**

diff --git a/ratis-metrics-default/src/main/java/org/apache/ratis/metrics/impl/MetricRegistriesImpl.java b/ratis-metrics-default/src/main/java/org/apache/ratis/metrics/impl/MetricRegistriesImpl.java
@@ -46,6 +46,7 @@ public class MetricRegistriesImpl extends MetricRegistries {
   private final MetricRegistryFactoryImpl factory;
 
   private final RefCountingMap<MetricRegistryInfo, RatisMetricRegistry> registries;
+  private final Object registerLock = new Object();
 
   public MetricRegistriesImpl() {
     this(new MetricRegistryFactoryImpl());
@@ -60,12 +61,17 @@ public MetricRegistriesImpl() {
   public RatisMetricRegistry create(MetricRegistryInfo info) {
     return registries.put(info, () -> {
       if (reporterRegistrations.isEmpty()) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("First MetricRegistry has been created without registering reporters. " +
-              "Hence registering JMX reporter by default.");
+        synchronized (registerLock) {
+          if (reporterRegistrations.isEmpty()) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug("First MetricRegistry has been created without registering reporters. " +
+                  "Hence registering JMX reporter by default.");
+            }
+            enableJmxReporter();
+          }
         }
-        enableJmxReporter();
       }
+
       RatisMetricRegistry registry = factory.create(info);
       reporterRegistrations.forEach(reg -> reg.accept(registry));
       return registry;

diff --git a/...opwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java b/...opwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java
@@ -47,6 +47,8 @@ public class Dm3MetricRegistriesImpl extends MetricRegistries {
 
   private final RefCountingMap<MetricRegistryInfo, RatisMetricRegistry> registries;
 
+  private final Object registerLock = new Object();
+
   public Dm3MetricRegistriesImpl() {
     this(new Dm3MetricRegistryFactoryImpl());
   }
@@ -60,11 +62,13 @@ public Dm3MetricRegistriesImpl() {
   public RatisMetricRegistry create(MetricRegistryInfo info) {
     return registries.put(info, () -> {
       if (reporterRegistrations.isEmpty()) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("First MetricRegistry has been created without registering reporters. " +
-              "Hence registering JMX reporter by default.");
+        synchronized (registerLock) {
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("First MetricRegistry has been created without registering reporters. " +
+                    "Hence registering JMX reporter by default.");
+          }
+          enableJmxReporter();
         }
-        enableJmxReporter();
       }
       RatisMetricRegistry registry = factory.create(info);
       reporterRegistrations.forEach(reg -> reg.accept(registry));