You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by yi...@apache.org on 2022/09/16 01:58:03 UTC

[hudi] branch master updated: [HUDI-4796] MetricsReporter stop bug (#6619)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new bf64e60d31 [HUDI-4796] MetricsReporter stop bug (#6619)
bf64e60d31 is described below

commit bf64e60d3121c67f51859be4ce42d04c6a5704be
Author: Tim Brown <ti...@onehouse.ai>
AuthorDate: Thu Sep 15 18:57:58 2022 -0700

    [HUDI-4796] MetricsReporter stop bug (#6619)
---
 .../org/apache/hudi/metrics/ConsoleMetricsReporter.java   | 15 ++++-----------
 .../org/apache/hudi/metrics/InMemoryMetricsReporter.java  |  7 -------
 .../java/org/apache/hudi/metrics/JmxMetricsReporter.java  |  7 -------
 .../src/main/java/org/apache/hudi/metrics/Metrics.java    | 15 ++++-----------
 .../org/apache/hudi/metrics/MetricsGraphiteReporter.java  |  6 ------
 .../java/org/apache/hudi/metrics/MetricsReporter.java     |  4 ----
 .../metrics/cloudwatch/CloudWatchMetricsReporter.java     |  6 ------
 .../hudi/metrics/datadog/DatadogMetricsReporter.java      |  6 ------
 .../hudi/metrics/prometheus/PrometheusReporter.java       | 13 ++++---------
 .../metrics/prometheus/PushGatewayMetricsReporter.java    | 11 +++--------
 .../apache/hudi/metrics/TestMetricsReporterFactory.java   |  6 ------
 11 files changed, 15 insertions(+), 81 deletions(-)

diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/ConsoleMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/ConsoleMetricsReporter.java
index b65c4ade88..5664240c62 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/ConsoleMetricsReporter.java
+++ b/hudi-client/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 class ConsoleMetricsReporter extends MetricsReporter {
     }
   }
 
-  @Override
-  public Closeable getReporter() {
-    return consoleReporter;
-  }
-
   @Override
   public void stop() {
     if (consoleReporter != null) {
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/InMemoryMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/InMemoryMetricsReporter.java
index a145024574..96439c3b31 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/InMemoryMetricsReporter.java
+++ b/hudi-client/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 class InMemoryMetricsReporter extends MetricsReporter {
   @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
index 309981a9d8..a909f62355 100644
--- 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 org.apache.log4j.LogManager;
 
 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 class JmxMetricsReporter extends MetricsReporter {
   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
index d9f22bca01..8f3e497481 100644
--- 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 com.codahale.metrics.MetricRegistry;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.util.Map;
 
 /**
@@ -56,14 +55,12 @@ public class Metrics {
     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 class Metrics {
     if (!initialized) {
       return;
     }
-    instance.reportAndCloseReporter();
+    instance.reportAndStopReporter();
     initialized = false;
   }
 
@@ -137,10 +134,6 @@ public class Metrics {
     return registry;
   }
 
-  public Closeable getReporter() {
-    return reporter.getReporter();
-  }
-
   public static boolean isInitialized() {
     return initialized;
   }
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java
index c6dff8fd86..34221f2c2f 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java
+++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsGraphiteReporter.java
@@ -27,7 +27,6 @@ import com.codahale.metrics.graphite.GraphiteReporter;
 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 class MetricsGraphiteReporter extends MetricsReporter {
     }
   }
 
-  @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
index 773bb3bfd8..64a0ae5613 100644
--- 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/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java
index c7134a1036..a0eb01abd0 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java
+++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/cloudwatch/CloudWatchMetricsReporter.java
@@ -26,7 +26,6 @@ import com.codahale.metrics.MetricRegistry;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import java.io.Closeable;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -72,11 +71,6 @@ public class CloudWatchMetricsReporter extends MetricsReporter {
     reporter.report();
   }
 
-  @Override
-  public Closeable getReporter() {
-    return reporter;
-  }
-
   @Override
   public void stop() {
     LOG.info("Stopping CloudWatch Metrics Reporter.");
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java
index 0830ef4c5b..fdbd0cc7e4 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java
+++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogMetricsReporter.java
@@ -28,7 +28,6 @@ import org.apache.hudi.metrics.datadog.DatadogHttpClient.ApiSite;
 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 class DatadogMetricsReporter extends MetricsReporter {
     reporter.report();
   }
 
-  @Override
-  public Closeable getReporter() {
-    return reporter;
-  }
-
   @Override
   public void stop() {
     reporter.stop();
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PrometheusReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PrometheusReporter.java
index 81c89b6e1c..1b53e9ea89 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PrometheusReporter.java
+++ b/hudi-client/hudi-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 class PrometheusReporter extends MetricsReporter {
   public void report() {
   }
 
-  @Override
-  public Closeable getReporter() {
-    return null;
-  }
-
   @Override
   public void stop() {
     collectorRegistry.unregister(metricExports);
diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PushGatewayMetricsReporter.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PushGatewayMetricsReporter.java
index fa4c947399..e2bfa6a67b 100644
--- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/prometheus/PushGatewayMetricsReporter.java
+++ b/hudi-client/hudi-client-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 class PushGatewayMetricsReporter extends MetricsReporter {
     pushGatewayReporter.report(null, null, null, null, null);
   }
 
-  @Override
-  public Closeable getReporter() {
-    return pushGatewayReporter;
-  }
-
   @Override
   public void stop() {
     pushGatewayReporter.stop();
diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
index 3689755e44..390f585ebb 100644
--- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
+++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
@@ -30,7 +30,6 @@ import org.junit.jupiter.api.extension.ExtendWith;
 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 class TestMetricsReporterFactory {
     @Override
     public void report() {}
 
-    @Override
-    public Closeable getReporter() {
-      return null;
-    }
-
     @Override
     public void stop() {}
   }