[HUDI-4796] MetricsReporter stop bug (apache#6619)

fengjian428 · Apr 5, 2023 · 943398b · 943398b
1 parent c139320
commit 943398b
Show file tree

Hide file tree

Showing 11 changed files with 15 additions and 81 deletions.
diff --git a/...ient/hudi-client-common/src/main/java/org/apache/hudi/metrics/ConsoleMetricsReporter.java b/...ient/hudi-client-common/src/main/java/org/apache/hudi/metrics/ConsoleMetricsReporter.java
@@ -18,15 +18,13 @@
 
 package org.apache.hudi.metrics;
 
-import java.io.Closeable;
-import java.util.concurrent.TimeUnit;
-
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
-
 import com.codahale.metrics.ConsoleReporter;
 import com.codahale.metrics.MetricFilter;
 import com.codahale.metrics.MetricRegistry;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import java.util.concurrent.TimeUnit;
 
 /**
  * Hudi Console metrics reporter. Reports the metrics by printing them to the stdout on the console.
@@ -61,11 +59,6 @@ public void report() {
     }
   }
 
-  @Override
-  public Closeable getReporter() {
-    return consoleReporter;
-  }
-
   @Override
   public void stop() {
     if (consoleReporter != null) {

diff --git a/...ent/hudi-client-common/src/main/java/org/apache/hudi/metrics/InMemoryMetricsReporter.java b/...ent/hudi-client-common/src/main/java/org/apache/hudi/metrics/InMemoryMetricsReporter.java
@@ -18,8 +18,6 @@
 
 package org.apache.hudi.metrics;
 
-import java.io.Closeable;
-
 /**
  * Used for testing.
  */
@@ -31,11 +29,6 @@ public void start() {}
   @Override
   public void report() {}
 
-  @Override
-  public Closeable getReporter() {
-    return null;
-  }
-
   @Override
   public void stop() {
 

diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java
@@ -26,12 +26,10 @@
 
 import javax.management.MBeanServer;
 
-import java.io.Closeable;
 import java.lang.management.ManagementFactory;
 import java.util.Objects;
 import java.util.stream.IntStream;
 
-
 /**
  * Implementation of Jmx reporter, which used to report jmx metric.
  */
@@ -86,11 +84,6 @@ public void start() {
   public void report() {
   }
 
-  @Override
-  public Closeable getReporter() {
-    return jmxReporterServer.getReporter();
-  }
-
   @Override
   public void stop() {
     Objects.requireNonNull(jmxReporterServer, "jmxReporterServer is not running.");

diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java
@@ -27,7 +27,6 @@
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.util.Map;
 
 /**
@@ -56,14 +55,12 @@ private Metrics(HoodieWriteConfig metricConfig) {
     Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
   }
 
-  private void reportAndCloseReporter() {
+  private void reportAndStopReporter() {
     try {
       registerHoodieCommonMetrics();
       reporter.report();
-      if (getReporter() != null) {
-        LOG.info("Closing metrics reporter...");
-        getReporter().close();
-      }
+      LOG.info("Stopping the metrics reporter...");
+      reporter.stop();
     } catch (Exception e) {
       LOG.warn("Error while closing reporter", e);
     }
@@ -105,7 +102,7 @@ public static synchronized void shutdown() {
     if (!initialized) {
       return;
     }
-    instance.reportAndCloseReporter();
+    instance.reportAndStopReporter();
     initialized = false;
   }
 
@@ -137,10 +134,6 @@ public MetricRegistry getRegistry() {
     return registry;
   }
 
-  public Closeable getReporter() {
-    return reporter.getReporter();
-  }
-
   public static boolean isInitialized() {
     return initialized;
   }

diff --git a/...ent/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java b/...ent/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java
@@ -27,7 +27,6 @@
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.net.InetSocketAddress;
 import java.util.concurrent.TimeUnit;
 
@@ -78,11 +77,6 @@ public void report() {
     }
   }
 
-  @Override
-  public Closeable getReporter() {
-    return graphiteReporter;
-  }
-
   private GraphiteReporter createGraphiteReport() {
     Graphite graphite = new Graphite(new InetSocketAddress(serverHost, serverPort));
     String reporterPrefix = config.getGraphiteMetricPrefix();

diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporter.java
@@ -18,8 +18,6 @@
 
 package org.apache.hudi.metrics;
 
-import java.io.Closeable;
-
 /**
  * Interface for implementing a Reporter.
  */
@@ -35,8 +33,6 @@ public abstract class MetricsReporter {
    */
   public abstract void report();
 
-  public abstract Closeable getReporter();
-
   /**
    * Stop this reporter. Should be used to stop channels, streams and release resources.
    */

diff --git a/...nt-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java b/...nt-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java
@@ -26,7 +26,6 @@
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -72,11 +71,6 @@ public void report() {
     reporter.report();
   }
 
-  @Override
-  public Closeable getReporter() {
-    return reporter;
-  }
-
   @Override
   public void stop() {
     LOG.info("Stopping CloudWatch Metrics Reporter.");

diff --git a/...i-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java b/...i-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java
@@ -28,7 +28,6 @@
 import com.codahale.metrics.MetricFilter;
 import com.codahale.metrics.MetricRegistry;
 
-import java.io.Closeable;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -81,11 +80,6 @@ public void report() {
     reporter.report();
   }
 
-  @Override
-  public Closeable getReporter() {
-    return reporter;
-  }
-
   @Override
   public void stop() {
     reporter.stop();

diff --git a/...di-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PrometheusReporter.java b/...di-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PrometheusReporter.java
@@ -18,17 +18,17 @@
 
 package org.apache.hudi.metrics.prometheus;
 
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.metrics.MetricsReporter;
+
 import com.codahale.metrics.MetricRegistry;
 import io.prometheus.client.CollectorRegistry;
 import io.prometheus.client.dropwizard.DropwizardExports;
 import io.prometheus.client.exporter.HTTPServer;
-import org.apache.hudi.config.HoodieWriteConfig;
-import org.apache.hudi.exception.HoodieException;
-import org.apache.hudi.metrics.MetricsReporter;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.net.InetSocketAddress;
 
 /**
@@ -65,11 +65,6 @@ public void start() {
   public void report() {
   }
 
-  @Override
-  public Closeable getReporter() {
-    return null;
-  }
-
   @Override
   public void stop() {
     collectorRegistry.unregister(metricExports);

diff --git a/...t-common/src/main/java/org/apache/hudi/metrics/prometheus/PushGatewayMetricsReporter.java b/...t-common/src/main/java/org/apache/hudi/metrics/prometheus/PushGatewayMetricsReporter.java
@@ -18,12 +18,12 @@
 
 package org.apache.hudi.metrics.prometheus;
 
-import com.codahale.metrics.MetricFilter;
-import com.codahale.metrics.MetricRegistry;
 import org.apache.hudi.config.HoodieWriteConfig;
 import org.apache.hudi.metrics.MetricsReporter;
 
-import java.io.Closeable;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 
@@ -65,11 +65,6 @@ public void report() {
     pushGatewayReporter.report(null, null, null, null, null);
   }
 
-  @Override
-  public Closeable getReporter() {
-    return pushGatewayReporter;
-  }
-
   @Override
   public void stop() {
     pushGatewayReporter.stop();

diff --git a/.../hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java b/.../hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
@@ -30,7 +30,6 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import java.io.Closeable;
 import java.util.Properties;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -87,11 +86,6 @@ public void start() {}
     @Override
     public void report() {}
 
-    @Override
-    public Closeable getReporter() {
-      return null;
-    }
-
     @Override
     public void stop() {}
   }