This is an automated email from the ASF dual-hosted git repository.

szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git


The following commit(s) were added to refs/heads/master by this push:
     new 95b51e512 RATIS-1705. Fix metrics leak (#744)
95b51e512 is described below

commit 95b51e512ffa3d0798607b82f8b474649413f2bd
Author: XiChen <[email protected]>
AuthorDate: Thu Aug 3 20:36:34 2023 +0800

    RATIS-1705. Fix metrics leak (#744)
---
 .../java/org/apache/ratis/grpc/metrics/MessageMetrics.java |  8 ++++++++
 .../apache/ratis/metrics/impl/MetricRegistriesImpl.java    | 14 ++++++++++----
 .../ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java | 12 ++++++++----
 3 files changed, 26 insertions(+), 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
index b152c6709..2a211aae8 100644
--- 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 @@ public class MessageMetrics extends RatisMetrics {
     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
index c46995921..17968ae9f 100644
--- 
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 class MetricRegistriesImpl extends MetricRegistries {
   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/ratis-metrics-dropwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java
 
b/ratis-metrics-dropwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java
index 9829e6126..b26f2e27a 100644
--- 
a/ratis-metrics-dropwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3/Dm3MetricRegistriesImpl.java
+++ 
b/ratis-metrics-dropwizard3/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 class Dm3MetricRegistriesImpl extends 
MetricRegistries {
   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));

Reply via email to