You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2021/03/29 19:26:09 UTC

[hbase] branch branch-1 updated: HBASE-25687 Backport "HBASE-25681 Add a switch for server/table queryMeter" to branch-2 and branch-1 (#3082)

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

stack pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-1 by this push:
     new 060e9e7  HBASE-25687 Backport "HBASE-25681 Add a switch for server/table queryMeter" to branch-2 and branch-1 (#3082)
060e9e7 is described below

commit 060e9e7cb1e2a9aad5b7e6a634e12cb6a183117b
Author: Baiqiang Zhao <zb...@gmail.com>
AuthorDate: Tue Mar 30 03:25:45 2021 +0800

    HBASE-25687 Backport "HBASE-25681 Add a switch for server/table queryMeter" to branch-2 and branch-1 (#3082)
    
    Signed-off-by: stack <st...@apache.org>
---
 .../hbase/regionserver/MetricsTableQueryMeter.java |  3 ++
 .../hadoop/hbase/test/MetricsAssertHelper.java     | 10 ++++
 .../regionserver/MetricsTableQueryMeterImpl.java   |  3 --
 .../hadoop/hbase/test/MetricsAssertHelperImpl.java |  9 +++-
 .../hbase/regionserver/MetricsRegionServer.java    | 29 +++++++++---
 .../regionserver/RegionServerTableMetrics.java     | 26 +++++-----
 .../regionserver/TestMetricsRegionServer.java      | 32 +++++++++++--
 .../regionserver/TestMetricsTableLatencies.java    | 55 +++++++++++++++++++++-
 8 files changed, 141 insertions(+), 26 deletions(-)

diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java
index fcce6e3..adb1584 100644
--- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java
+++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java
@@ -25,6 +25,9 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience;
 @InterfaceAudience.Private
 public interface MetricsTableQueryMeter {
 
+  String TABLE_READ_QUERY_PER_SECOND = "tableReadQueryPerSecond";
+  String TABLE_WRITE_QUERY_PER_SECOND = "tableWriteQueryPerSecond";
+
   /**
    * Update table read QPS
    * @param tableName The table the metric is for
diff --git a/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java b/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
index 52e0d09..0649fc9 100644
--- a/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
+++ b/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
@@ -150,6 +150,16 @@ public interface MetricsAssertHelper {
   boolean checkCounterExists(String name, BaseSource source);
 
   /**
+   * Check if a gauge exists.
+   *
+   * @param name   name of the gauge.
+   * @param source The BaseSource{@link BaseSource} that will provide the tags,
+   *               gauges, and counters.
+   * @return boolean true if gauge metric exists.
+   */
+  boolean checkGaugeExists(String name, BaseSource source);
+
+  /**
    * Get the value of a gauge as a double.
    *
    * @param name   name of the gauge.
diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java
index cd3526a..babaa66 100644
--- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java
+++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java
@@ -33,9 +33,6 @@ public class MetricsTableQueryMeterImpl implements MetricsTableQueryMeter {
   private final Map<TableName, TableMeters> metersByTable = new ConcurrentHashMap<>();
   private final MetricRegistry metricRegistry;
 
-  private final static String TABLE_READ_QUERY_PER_SECOND = "tableReadQueryPerSecond";
-  private final static String TABLE_WRITE_QUERY_PER_SECOND = "tableWriteQueryPerSecond";
-
   public MetricsTableQueryMeterImpl(MetricRegistry metricRegistry) {
     this.metricRegistry = metricRegistry;
   }
diff --git a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
index 117a3e9..7a86016 100644
--- a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
+++ b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
@@ -209,7 +209,14 @@ public class MetricsAssertHelperImpl implements MetricsAssertHelper {
   public boolean checkCounterExists(String name, BaseSource source) {
     getMetrics(source);
     String cName = canonicalizeMetricName(name);
-    return (counters.get(cName) != null) ? true : false;
+    return counters.get(cName) != null;
+  }
+
+  @Override
+  public boolean checkGaugeExists(String name, BaseSource source) {
+    getMetrics(source);
+    String cName = canonicalizeMetricName(name);
+    return gauges.get(cName) != null;
   }
   
   @Override
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
index cd725d0..a724bb1 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
@@ -40,6 +40,12 @@ public class MetricsRegionServer {
   public static final String RS_ENABLE_TABLE_METRICS_KEY =
       "hbase.regionserver.enable.table.latencies";
   public static final boolean RS_ENABLE_TABLE_METRICS_DEFAULT = true;
+  public static final String RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY =
+    "hbase.regionserver.enable.server.query.meter";
+  public static final boolean RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY_DEFAULT = true;
+  public static final String RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY =
+    "hbase.regionserver.enable.table.query.meter";
+  public static final boolean RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT = true;
 
   private final MetricsRegionServerSource serverSource;
   private final MetricsRegionServerWrapper regionServerWrapper;
@@ -65,8 +71,11 @@ public class MetricsRegionServer {
 
     // create and use metrics from the new hbase-metrics based registry.
     bulkLoadTimer = metricRegistry.timer("Bulkload");
-    serverReadQueryMeter = metricRegistry.meter("ServerReadQueryPerSecond");
-    serverWriteQueryMeter = metricRegistry.meter("ServerWriteQueryPerSecond");
+    if (conf.getBoolean(RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY,
+      RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY_DEFAULT)) {
+      serverReadQueryMeter = metricRegistry.meter("ServerReadQueryPerSecond");
+      serverWriteQueryMeter = metricRegistry.meter("ServerWriteQueryPerSecond");
+    }
   }
 
   MetricsRegionServer(MetricsRegionServerWrapper regionServerWrapper,
@@ -83,7 +92,9 @@ public class MetricsRegionServer {
    */
   static RegionServerTableMetrics createTableMetrics(Configuration conf) {
     if (conf.getBoolean(RS_ENABLE_TABLE_METRICS_KEY, RS_ENABLE_TABLE_METRICS_DEFAULT)) {
-      return new RegionServerTableMetrics();
+      return new RegionServerTableMetrics(
+        conf.getBoolean(RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY,
+          RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT));
     }
     return null;
   }
@@ -239,20 +250,26 @@ public class MetricsRegionServer {
     if (tableMetrics != null && tn != null) {
       tableMetrics.updateTableReadQueryMeter(tn, count);
     }
-    this.serverReadQueryMeter.mark(count);
+    if (serverReadQueryMeter != null) {
+      serverReadQueryMeter.mark(count);
+    }
   }
 
   public void updateWriteQueryMeter(TableName tn, long count) {
     if (tableMetrics != null && tn != null) {
       tableMetrics.updateTableWriteQueryMeter(tn, count);
     }
-    this.serverWriteQueryMeter.mark(count);
+    if (serverWriteQueryMeter != null) {
+      serverWriteQueryMeter.mark(count);
+    }
   }
 
   public void updateWriteQueryMeter(TableName tn) {
     if (tableMetrics != null && tn != null) {
       tableMetrics.updateTableWriteQueryMeter(tn);
     }
-    this.serverWriteQueryMeter.mark();
+    if (serverWriteQueryMeter != null) {
+      serverWriteQueryMeter.mark();
+    }
   }
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
index ab68eb3..bc3f9f0 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
@@ -29,12 +29,14 @@ import org.apache.hadoop.hbase.metrics.MetricRegistries;
 public class RegionServerTableMetrics {
 
   private final MetricsTableLatencies latencies;
-  private final MetricsTableQueryMeter queryMeter;
+  private MetricsTableQueryMeter queryMeter;
 
-  public RegionServerTableMetrics() {
+  public RegionServerTableMetrics(boolean enableTableQueryMeter) {
     latencies = CompatibilitySingletonFactory.getInstance(MetricsTableLatencies.class);
-    queryMeter = new MetricsTableQueryMeterImpl(MetricRegistries.global().
-      get(((MetricsTableLatenciesImpl) latencies).getMetricRegistryInfo()).get());
+    if (enableTableQueryMeter) {
+      queryMeter = new MetricsTableQueryMeterImpl(MetricRegistries.global().
+        get(((MetricsTableLatenciesImpl) latencies).getMetricRegistryInfo()).get());
+    }
   }
 
   public void updatePut(TableName table, long time) {
@@ -82,18 +84,20 @@ public class RegionServerTableMetrics {
   }
 
   public void updateTableReadQueryMeter(TableName table, long count) {
-    queryMeter.updateTableReadQueryMeter(table, count);
-  }
-
-  public void updateTableReadQueryMeter(TableName table) {
-    queryMeter.updateTableReadQueryMeter(table);
+    if (queryMeter != null) {
+      queryMeter.updateTableReadQueryMeter(table, count);
+    }
   }
 
   public void updateTableWriteQueryMeter(TableName table, long count) {
-    queryMeter.updateTableWriteQueryMeter(table, count);
+    if (queryMeter != null) {
+      queryMeter.updateTableWriteQueryMeter(table, count);
+    }
   }
 
   public void updateTableWriteQueryMeter(TableName table) {
-    queryMeter.updateTableWriteQueryMeter(table);
+    if (queryMeter != null) {
+      queryMeter.updateTableWriteQueryMeter(table);
+    }
   }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
index 1096657..ae72a8d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
@@ -17,8 +17,13 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.test.MetricsAssertHelper;
 import org.apache.hadoop.hbase.util.JvmPauseMonitor;
@@ -27,8 +32,6 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import static org.junit.Assert.assertNotNull;
-
 /**
  * Unit test version of rs metrics tests.
  */
@@ -49,7 +52,9 @@ public class TestMetricsRegionServer {
   @Before
   public void setUp() {
     wrapper = new MetricsRegionServerWrapperStub();
-    rsm = new MetricsRegionServer(wrapper, new Configuration(false));
+    Configuration conf = new Configuration(false);
+    conf.setBoolean(MetricsRegionServer.RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY, false);
+    rsm = new MetricsRegionServer(wrapper, conf);
     serverSource = rsm.getMetricsSource();
   }
 
@@ -233,5 +238,26 @@ public class TestMetricsRegionServer {
     HELPER.assertCounter("pauseTimeWithGc_num_ops", 1, serverSource);
   }
 
+  @Test
+  public void testServerQueryMeterSwitch() {
+    TableName tn1 = TableName.valueOf("table1");
+    // has been set disable in setUp()
+    rsm.updateReadQueryMeter(tn1, 500L);
+    assertFalse(HELPER.checkGaugeExists("ServerReadQueryPerSecond_count", serverSource));
+    rsm.updateWriteQueryMeter(tn1, 500L);
+    assertFalse(HELPER.checkGaugeExists("ServerWriteQueryPerSecond_count", serverSource));
+
+    // enable
+    Configuration conf = new Configuration(false);
+    conf.setBoolean(MetricsRegionServer.RS_ENABLE_SERVER_QUERY_METER_METRICS_KEY, true);
+    rsm = new MetricsRegionServer(wrapper, conf);
+    serverSource = rsm.getMetricsSource();
+    rsm.updateReadQueryMeter(tn1, 500L);
+    assertTrue(HELPER.checkGaugeExists("ServerWriteQueryPerSecond_count", serverSource));
+    HELPER.assertGauge("ServerReadQueryPerSecond_count", 500L, serverSource);
+    assertTrue(HELPER.checkGaugeExists("ServerWriteQueryPerSecond_count", serverSource));
+    rsm.updateWriteQueryMeter(tn1, 500L);
+    HELPER.assertGauge("ServerWriteQueryPerSecond_count", 500L, serverSource);
+  }
 }
 
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
index 5bd0e9d..12c27bd 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
@@ -17,10 +17,12 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CompatibilityFactory;
 import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
 import org.apache.hadoop.hbase.TableName;
@@ -45,7 +47,7 @@ public class TestMetricsTableLatencies {
     assertTrue("'latencies' is actually " + latencies.getClass(),
         latencies instanceof MetricsTableLatenciesImpl);
     MetricsTableLatenciesImpl latenciesImpl = (MetricsTableLatenciesImpl) latencies;
-    RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics();
+    RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics(false);
 
     // Metrics to each table should be disjoint
     // N.B. each call to assertGauge removes all previously acquired metrics so we have to
@@ -66,4 +68,53 @@ public class TestMetricsTableLatencies {
     HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
         tn2, MetricsTableLatencies.PUT_TIME + "_" + "99th_percentile"), 75L, latenciesImpl);
   }
-}
\ No newline at end of file
+
+  @Test
+  public void testTableQueryMeterSwitch() {
+    TableName tn1 = TableName.valueOf("table1");
+    MetricsTableLatencies latencies = CompatibilitySingletonFactory.getInstance(
+      MetricsTableLatencies.class);
+    assertTrue("'latencies' is actually " + latencies.getClass(),
+      latencies instanceof MetricsTableLatenciesImpl);
+    MetricsTableLatenciesImpl latenciesImpl = (MetricsTableLatenciesImpl) latencies;
+
+    Configuration conf = new Configuration();
+    conf.setBoolean(MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY, false);
+    boolean enableTableQueryMeter = conf.getBoolean(
+      MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY,
+      MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT);
+    // disable
+    assertFalse(enableTableQueryMeter);
+    RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics(enableTableQueryMeter);
+    tableMetrics.updateTableReadQueryMeter(tn1, 500L);
+    assertFalse(HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_READ_QUERY_PER_SECOND + "_" + "count"),
+      latenciesImpl));
+    tableMetrics.updateTableWriteQueryMeter(tn1, 500L);
+    assertFalse(HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND + "_" + "count"),
+      latenciesImpl));
+
+    // enable
+    conf.setBoolean(MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY, true);
+    enableTableQueryMeter = conf.getBoolean(
+      MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY,
+      MetricsRegionServer.RS_ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT);
+    assertTrue(enableTableQueryMeter);
+    tableMetrics = new RegionServerTableMetrics(true);
+    tableMetrics.updateTableReadQueryMeter(tn1, 500L);
+    assertTrue(HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_READ_QUERY_PER_SECOND + "_" + "count"),
+      latenciesImpl));
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_READ_QUERY_PER_SECOND + "_" + "count"),
+      500L, latenciesImpl);
+    tableMetrics.updateTableWriteQueryMeter(tn1, 500L);
+    assertTrue(HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND + "_" + "count"),
+      latenciesImpl));
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+      tn1, MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND + "_" + "count"),
+      500L, latenciesImpl);
+  }
+}