[GitHub] [hudi] nsivabalan commented on a diff in pull request #7934: [HUDI-5777] Support Metrics for Multiple Tables Simultaneously

["nsivabalan (via GitHub)" <gi...@apache.org>]

nsivabalan commented on code in PR #7934:
URL: https://github.com/apache/hudi/pull/7934#discussion_r1108725732


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -52,75 +52,62 @@ private Metrics(HoodieWriteConfig metricConfig) {
     }
     reporter.start();
 
-    Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
+    Runtime.getRuntime().addShutdownHook(new Thread(this::shutdown));
+    this.initialized = true;
   }
 
-  private void reportAndStopReporter() {
-    try {
-      registerHoodieCommonMetrics();
-      reporter.report();
-      LOG.info("Stopping the metrics reporter...");
-      reporter.stop();
-    } catch (Exception e) {
-      LOG.warn("Error while closing reporter", e);
-    }
+  private void registerHoodieCommonMetrics() {
+    registerGauges(Registry.getAllMetrics(true, true), Option.of(commonMetricPrefix));
   }
 
-  private void reportAndFlushMetrics() {
-    try {
-      LOG.info("Reporting and flushing all metrics");
-      this.registerHoodieCommonMetrics();
-      this.reporter.report();
-      this.registry.getNames().forEach(this.registry::remove);
-    } catch (Exception e) {
-      LOG.error("Error while reporting and flushing metrics", e);
+  public static synchronized Metrics getInstance(HoodieWriteConfig metricConfig) {
+    String basePath = metricConfig.getBasePath();
+    if (METRICS_INSTANCE_PER_BASEPATH.containsKey(basePath)) {
+      return METRICS_INSTANCE_PER_BASEPATH.get(basePath);
     }
-  }
 
-  private void registerHoodieCommonMetrics() {
-    registerGauges(Registry.getAllMetrics(true, true), Option.of(commonMetricPrefix));
+    Metrics metrics = new Metrics(metricConfig);
+    METRICS_INSTANCE_PER_BASEPATH.put(basePath, metrics);
+    return metrics;
   }
 
-  public static Metrics getInstance() {
-    assert initialized;
-    return instance;
+  public static synchronized void shutdownAllMetrics() {
+    METRICS_INSTANCE_PER_BASEPATH.values().forEach(Metrics::shutdown);
   }
 
-  public static synchronized void init(HoodieWriteConfig metricConfig) {
-    if (initialized) {
-      return;
-    }
+  public synchronized void shutdown() {
     try {
-      instance = new Metrics(metricConfig);
+      registerHoodieCommonMetrics();
+      reporter.report();
+      LOG.info("Stopping the metrics reporter...");
+      if (reporter != null) {

Review Comment:
   how can reporter could be null. Just 1 line above we are calling reporter.report() and this is within synchronized method. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org