This is an automated email from the ASF dual-hosted git repository.
jlli pushed a commit to branch hotfix-minmax
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/hotfix-minmax by this push:
new 0a65ba9 Fix race condition in MetricsHelper (#5887)
0a65ba9 is described below
commit 0a65ba95abb20af1f7e8fb2cec8c61aa8c4a73e2
Author: Jialiang Li <j...@linkedin.com>
AuthorDate: Mon Aug 17 17:52:01 2020 -0700
Fix race condition in MetricsHelper (#5887)
Fix race condition in MetricsHelper
Bump up dropwizard metrics version to 4.1.2
Co-authored-by: Jack Li(Analytics Engineering) <j...@jlli-mn1.linkedin.biz>
---
.../org/apache/pinot/common/metrics/AbstractMetrics.java | 2 ++
.../JmxReporterMetricsRegistryRegistrationListener.java | 6 ++++++
.../org/apache/pinot/common/metrics/MetricsHelper.java | 15 +++++++++------
pom.xml | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
index 5178fc1..cf40f69 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
@@ -376,6 +376,7 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
*/
public void initializeGlobalMeters() {
M[] meters = getMeters();
+ LOGGER.info("Initializing global {} meters", meters.length);
for (M meter : meters) {
if (meter.isGlobal()) {
@@ -384,6 +385,7 @@ public abstract class AbstractMetrics<QP extends
AbstractMetrics.QueryPhase, M e
}
G[] gauges = getGauges();
+ LOGGER.info("Initializing global {} gauges", gauges.length);
for (G gauge : gauges) {
if (gauge.isGlobal()) {
setValueOfGlobalGauge(gauge, 0);
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/JmxReporterMetricsRegistryRegistrationListener.java
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/JmxReporterMetricsRegistryRegistrationListener.java
index 1c2cf15..9e6cb13 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/JmxReporterMetricsRegistryRegistrationListener.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/JmxReporterMetricsRegistryRegistrationListener.java
@@ -20,6 +20,8 @@ package org.apache.pinot.common.metrics;
import com.yammer.metrics.core.MetricsRegistry;
import com.yammer.metrics.reporting.JmxReporter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
@@ -27,8 +29,12 @@ import com.yammer.metrics.reporting.JmxReporter;
*
*/
public class JmxReporterMetricsRegistryRegistrationListener implements
MetricsRegistryRegistrationListener {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(JmxReporterMetricsRegistryRegistrationListener.class);
+
@Override
public void onMetricsRegistryRegistered(MetricsRegistry metricsRegistry) {
+ LOGGER.info("Registering JmxReporterMetricsRegistryRegistrationListener");
new JmxReporter(metricsRegistry).start();
+ LOGGER.info("Number of metrics in metricsRegistry: {}",
metricsRegistry.allMetrics().size());
}
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/MetricsHelper.java
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/MetricsHelper.java
index 9d2933f..f6e2eb1 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/metrics/MetricsHelper.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/metrics/MetricsHelper.java
@@ -34,7 +34,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.slf4j.Logger;
@@ -44,10 +44,10 @@ import org.slf4j.LoggerFactory;
public class MetricsHelper {
private static final Logger LOGGER =
LoggerFactory.getLogger(MetricsHelper.class);
- private static Map<MetricsRegistry, Object> metricsRegistryMap = new
WeakHashMap<MetricsRegistry, Object>();
+ private static Map<MetricsRegistry, Boolean> metricsRegistryMap = new
ConcurrentHashMap<>();
- private static Map<MetricsRegistryRegistrationListener, Object>
metricsRegistryRegistrationListenersMap =
- new WeakHashMap<MetricsRegistryRegistrationListener, Object>();
+ private static Map<MetricsRegistryRegistrationListener, Boolean>
metricsRegistryRegistrationListenersMap =
+ new ConcurrentHashMap<>();
/**
* Initializes the metrics system by initializing the registry registration
listeners present in the configuration.
@@ -68,6 +68,7 @@ public class MetricsHelper {
clazz.getDeclaredConstructor();
MetricsRegistryRegistrationListener listener =
defaultConstructor.newInstance();
+ LOGGER.info("Registering metricsRegistry to listener {}",
listenerClassName);
addMetricsRegistryRegistrationListener(listener);
} catch (Exception e) {
LOGGER
@@ -75,6 +76,7 @@ public class MetricsHelper {
}
}
}
+ LOGGER.info("Number of listeners got registered: {}",
metricsRegistryRegistrationListenersMap.size());
}
/**
@@ -86,10 +88,11 @@ public class MetricsHelper {
*/
public static void
addMetricsRegistryRegistrationListener(MetricsRegistryRegistrationListener
listener) {
synchronized (MetricsHelper.class) {
- metricsRegistryRegistrationListenersMap.put(listener, null);
+ metricsRegistryRegistrationListenersMap.put(listener, Boolean.TRUE);
// Fire events to register all previously registered metrics registries
Set<MetricsRegistry> metricsRegistries = metricsRegistryMap.keySet();
+ LOGGER.info("Number of metrics registry: {}", metricsRegistries.size());
for (MetricsRegistry metricsRegistry : metricsRegistries) {
listener.onMetricsRegistryRegistered(metricsRegistry);
}
@@ -103,7 +106,7 @@ public class MetricsHelper {
*/
public static void registerMetricsRegistry(MetricsRegistry registry) {
synchronized (MetricsHelper.class) {
- metricsRegistryMap.put(registry, null);
+ metricsRegistryMap.put(registry, Boolean.TRUE);
// Fire event to all registered listeners
Set<MetricsRegistryRegistrationListener>
metricsRegistryRegistrationListeners =
diff --git a/pom.xml b/pom.xml
index c59189a..0b1be4d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -139,7 +139,7 @@
<!-- hadoop-common, spark-core use commons-net -->
<commons-net.version>3.1</commons-net.version>
<!-- helix-core, spark-core use libraries from io.dropwizard.metrics -->
- <dropwizard-metrics.version>3.2.3</dropwizard-metrics.version>
+ <dropwizard-metrics.version>4.1.2</dropwizard-metrics.version>
<snappy-java.version>1.1.1.7</snappy-java.version>
<log4j.version>2.11.2</log4j.version>
<netty.version>4.1.42.Final</netty.version>
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
