You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ne...@apache.org on 2021/10/26 00:52:38 UTC

[pinot] branch master updated: Push metricsRegistry init into synchronized get call (#7636)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new ed335ef  Push metricsRegistry init into synchronized get call (#7636)
ed335ef is described below

commit ed335ef91b5082cfc19be0adef08bb577cc29ebe
Author: Neha Pawar <ne...@gmail.com>
AuthorDate: Mon Oct 25 17:52:24 2021 -0700

    Push metricsRegistry init into synchronized get call (#7636)
---
 .../broker/broker/helix/BaseBrokerStarter.java      |  4 +---
 .../pinot/common/metrics/PinotMetricUtils.java      | 21 +++++++++++++++++----
 .../pinot/controller/BaseControllerStarter.java     |  4 +---
 .../org/apache/pinot/minion/BaseMinionStarter.java  |  4 +---
 .../apache/pinot/server/starter/ServerInstance.java |  4 +---
 5 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
index 238056a..dcdb700 100644
--- a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
+++ b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
@@ -199,9 +199,7 @@ public abstract class BaseBrokerStarter implements ServiceStartable {
 
     LOGGER.info("Setting up broker request handler");
     // Set up metric registry and broker metrics
-    PinotConfiguration metricsConfiguration = _brokerConf.subset(Broker.METRICS_CONFIG_PREFIX);
-    PinotMetricUtils.init(metricsConfiguration);
-    _metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry();
+    _metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(_brokerConf.subset(Broker.METRICS_CONFIG_PREFIX));
     _brokerMetrics = new BrokerMetrics(
         _brokerConf.getProperty(Broker.CONFIG_OF_METRICS_NAME_PREFIX, Broker.DEFAULT_METRICS_NAME_PREFIX),
         _metricsRegistry,
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
index 81f0096..df08f91 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.common.metrics;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import java.lang.reflect.Constructor;
 import java.util.Arrays;
@@ -58,7 +59,11 @@ public class PinotMetricUtils {
 
   private static PinotMetricsFactory _pinotMetricsFactory = null;
 
-  public static void init(PinotConfiguration metricsConfiguration) {
+  /**
+   * Initialize the metricsFactory ad registers the metricsRegistry
+   */
+  @VisibleForTesting
+  public synchronized static void init(PinotConfiguration metricsConfiguration) {
     // Initializes PinotMetricsFactory.
     initializePinotMetricsFactory(metricsConfiguration);
 
@@ -185,11 +190,19 @@ public class PinotMetricUtils {
     _pinotMetricsFactory = metricsFactory;
   }
 
+  @VisibleForTesting
   public static PinotMetricsRegistry getPinotMetricsRegistry() {
+    return getPinotMetricsRegistry(new PinotConfiguration(Collections.emptyMap()));
+  }
+
+  /**
+   * Returns the metricsRegistry from the initialised metricsFactory.
+   * If the metricsFactory is null, first creates and initializes the metricsFactory and registers the metrics registry.
+   * @param metricsConfiguration metrics configs
+   */
+  public static synchronized PinotMetricsRegistry getPinotMetricsRegistry(PinotConfiguration metricsConfiguration) {
     if (_pinotMetricsFactory == null) {
-      // If init method didn't get called previously, just simply init with an empty hashmap. This is commonly used
-      // in tests.
-      init(new PinotConfiguration(Collections.emptyMap()));
+      init(metricsConfiguration);
     }
     return _pinotMetricsFactory.getPinotMetricsRegistry();
   }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
index 24e8f1c..2b29fa2 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
@@ -520,9 +520,7 @@ public abstract class BaseControllerStarter implements ServiceStartable {
   }
 
   private void initControllerMetrics() {
-    PinotConfiguration metricsConfiguration = _config.subset(METRICS_REGISTRY_NAME);
-    PinotMetricUtils.init(metricsConfiguration);
-    _metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry();
+    _metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(_config.subset(METRICS_REGISTRY_NAME));
     _controllerMetrics = new ControllerMetrics(_config.getMetricsPrefix(), _metricsRegistry);
     _controllerMetrics.initializeGlobalMeters();
   }
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
index aa55d1e..86e74f2 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
@@ -172,9 +172,7 @@ public abstract class BaseMinionStarter implements ServiceStartable {
     // Initialize metrics
     LOGGER.info("Initializing metrics");
     // TODO: put all the metrics related configs down to "pinot.server.metrics"
-    PinotConfiguration metricsConfiguration = _config.getMetricsConfig();
-    PinotMetricUtils.init(metricsConfiguration);
-    PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry();
+    PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(_config.getMetricsConfig());
 
     MinionMetrics minionMetrics = new MinionMetrics(_config
         .getProperty(CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX_KEY,
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/ServerInstance.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/ServerInstance.java
index 031a279..95062ee 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/starter/ServerInstance.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/starter/ServerInstance.java
@@ -67,9 +67,7 @@ public class ServerInstance {
     LOGGER.info("Initializing server instance");
 
     LOGGER.info("Initializing server metrics");
-    PinotConfiguration metricsConfiguration = serverConf.getMetricsConfig();
-    PinotMetricUtils.init(metricsConfiguration);
-    PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry();
+    PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(serverConf.getMetricsConfig());
     _serverMetrics =
         new ServerMetrics(serverConf.getMetricsPrefix(), metricsRegistry, serverConf.emitTableLevelMetrics(),
             serverConf.getAllowedTablesForEmittingMetrics());

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org