[phoenix] branch master updated: PHOENIX-6699 Phoenix metrics overwriting DefaultMetricsSystem in RegionServers

[stoty]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 58b6935884 PHOENIX-6699 Phoenix metrics overwriting 
DefaultMetricsSystem in RegionServers
58b6935884 is described below

commit 58b693588403a576334460f13612efd94d0c7e64
Author: Istvan Toth <st...@apache.org>
AuthorDate: Thu Apr 28 07:21:33 2022 +0200

    PHOENIX-6699 Phoenix metrics overwriting DefaultMetricsSystem in 
RegionServers
---
 .../monitoring/GlobalMetricRegistriesAdapter.java  |  7 ++++++
 .../org/apache/phoenix/monitoring/MetricUtil.java  | 26 ++++++++++++++++++++++
 .../apache/phoenix/monitoring/MetricUtilTest.java  | 17 ++++++++++++++
 3 files changed, 50 insertions(+)

diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalMetricRegistriesAdapter.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalMetricRegistriesAdapter.java
index 0f1c8c2744..4a9093511f 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalMetricRegistriesAdapter.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalMetricRegistriesAdapter.java
@@ -53,6 +53,13 @@ public class GlobalMetricRegistriesAdapter {
     private static GlobalMetricRegistriesAdapter INSTANCE = new 
GlobalMetricRegistriesAdapter();
 
     private GlobalMetricRegistriesAdapter() {
+        if (MetricUtil.isDefaultMetricsInitialized()) {
+            // Prevent clobbering the default metrics HBase has set up in
+            // RS or Master while JmxCacheBuster shuts the Metrics down
+            LOGGER.info("HBase metrics is already initialized. "
+                    + "Skipping Phoenix metrics initialization.");
+            return;
+        }
         DefaultMetricsSystem.initialize("Phoenix");
         JvmMetrics.initSingleton("Phoenix", "");
     }
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricUtil.java 
b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricUtil.java
index 1974eb806e..bbe5f2a46e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricUtil.java
@@ -17,11 +17,19 @@
  */
 package org.apache.phoenix.monitoring;
 
+import java.lang.reflect.Field;
+
+import org.apache.hadoop.metrics2.impl.MetricsSystemImpl;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
 import org.apache.phoenix.log.LogLevel;
 import org.apache.phoenix.monitoring.CombinableMetric.NoOpRequestMetric;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class MetricUtil {
 
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(MetricUtil.class);
+
     public static CombinableMetric getCombinableMetric(boolean 
isRequestMetricsEnabled,
                                                        LogLevel 
connectionLogLevel,
                                                        MetricType type) {
@@ -38,4 +46,22 @@ public class MetricUtil {
         return new MetricsStopWatch(true);
     }
 
+    // We need to cover the case when JmxCacheBuster has just stopped the 
HBase metrics
+    // system, and not accidentally overwrite the DefaultMetricsSystem 
singleton.
+    // See PHOENIX-6699
+    public static boolean isDefaultMetricsInitialized() {
+        try {
+            MetricsSystemImpl metrics = (MetricsSystemImpl) 
DefaultMetricsSystem.instance();
+            Field prefixField = 
MetricsSystemImpl.class.getDeclaredField("prefix");
+            prefixField.setAccessible(true);
+            String prefix = (String) prefixField.get(metrics);
+            prefixField.setAccessible(false);
+            if (prefix != null) {
+                return true;
+            }
+        } catch (Exception e) {
+            LOGGER.error("Exception trying to determine if HBase metrics is 
initialized", e);
+        }
+        return false;
+    }
 }
diff --git 
a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/MetricUtilTest.java 
b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/MetricUtilTest.java
index 141ce724d1..3e44ecfceb 100644
--- 
a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/MetricUtilTest.java
+++ 
b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/MetricUtilTest.java
@@ -17,6 +17,8 @@
  */
 package org.apache.phoenix.monitoring;
 
+import org.apache.hadoop.metrics2.impl.MetricsSystemImpl;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
 import org.apache.phoenix.log.LogLevel;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -27,6 +29,8 @@ import static 
org.apache.phoenix.monitoring.MetricType.WALL_CLOCK_TIME_MS;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.lang.reflect.Field;
+
 @RunWith(MockitoJUnitRunner.class)
 public class MetricUtilTest {
 
@@ -48,4 +52,17 @@ public class MetricUtilTest {
                 LogLevel.OFF, WALL_CLOCK_TIME_MS);
         assertFalse(metricsStopWatch.getMetricsEnabled());
     }
+
+    @Test
+    //Check that MetricsSystemImpl has a String "prefix" field in the Hadoop 
version we test with
+    public void testInternalMetricsField() throws NoSuchFieldException,
+            SecurityException, IllegalArgumentException, 
IllegalAccessException {
+        MetricsSystemImpl metrics = (MetricsSystemImpl) 
DefaultMetricsSystem.instance();
+        Field prefixField = MetricsSystemImpl.class.getDeclaredField("prefix");
+        prefixField.setAccessible(true);
+        String oldValue = (String)prefixField.get(metrics);
+        prefixField.set(metrics, "dummy");
+        prefixField.set(metrics, oldValue);
+        prefixField.setAccessible(false);
+    }
 }