HBASE-27681 Refactor Table Latency Metrics (#5072)

Signed-off-by: Bryan Beaudreault <[email protected]>

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/impl/GlobalMetricRegistriesAdapter.java

@@ -115,6 +115,7 @@ private void doRun() {
     for (MetricRegistry registry : registries) {
       MetricRegistryInfo info = registry.getMetricRegistryInfo();
 
+      LOG.trace("MetricRegistryInfo : " + info.getMetricsName());
       if (info.isExistingSource()) {
         // If there is an already existing BaseSource for this MetricRegistry, skip it here. These
         // types of registries are there only due to existing BaseSource implementations in the

hbase-metrics/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestMetricRegistriesImpl.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.metrics.impl;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.Optional;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test class for {@link MetricRegistries}.
+ */
+@Category(SmallTests.class)
+public class TestMetricRegistriesImpl {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestMetricRegistriesImpl.class);
+
+  @Test
+  public void testMetricsRegistriesRemoveRef() {
+    MetricRegistryInfo registryInfo =
+      new MetricRegistryInfo("testMetrics", null, null, null, false);
+    MetricRegistries.global().create(registryInfo);
+    Optional<MetricRegistry> registry1 = MetricRegistries.global().get(registryInfo);
+    assertTrue(registry1.isPresent());
+
+    MetricRegistries.global().create(registryInfo);
+    Optional<MetricRegistry> registry2 = MetricRegistries.global().get(registryInfo);
+    assertTrue(registry2.isPresent());
+
+    MetricRegistries.global().remove(registryInfo);
+    Optional<MetricRegistry> registry3 = MetricRegistries.global().get(registryInfo);
+    assertTrue(registry3.isPresent());
+
+    MetricRegistries.global().remove(registryInfo);
+    Optional<MetricRegistry> registry4 = MetricRegistries.global().get(registryInfo);
+    assertTrue(!registry4.isPresent());
+  }
+}

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

@@ -148,6 +148,7 @@
 import org.apache.hadoop.hbase.regionserver.compactions.CompactionContext;
 import org.apache.hadoop.hbase.regionserver.compactions.CompactionLifeCycleTracker;
 import org.apache.hadoop.hbase.regionserver.compactions.ForbidMajorCompactionChecker;
+import org.apache.hadoop.hbase.regionserver.metrics.MetricsTableRequests;
 import org.apache.hadoop.hbase.regionserver.regionreplication.RegionReplicationSink;
 import org.apache.hadoop.hbase.regionserver.throttle.CompactionThroughputControllerFactory;
 import org.apache.hadoop.hbase.regionserver.throttle.NoLimitThroughputController;
@@ -373,6 +374,13 @@ public void setRestoredRegion(boolean restoredRegion) {
     isRestoredRegion = restoredRegion;
   }
 
+  public MetricsTableRequests getMetricsTableRequests() {
+    return metricsTableRequests;
+  }
+
+  // Handle table latency metrics
+  private MetricsTableRequests metricsTableRequests;
+
   // The internal wait duration to acquire a lock before read/update
   // from the region. It is not per row. The purpose of this wait time
   // is to avoid waiting a long time while the region is busy, so that
@@ -962,6 +970,9 @@ long initialize(final CancelableProgressable reporter) throws IOException {
         }
 
       }
+      if (metricsTableRequests != null) {
+        metricsTableRequests.removeRegistry();
+      }
       throw e;
     } finally {
       // nextSeqid will be -1 if the initialization fails.
@@ -1091,6 +1102,9 @@ private long initializeRegionInternals(final CancelableProgressable reporter,
       status.setStatus("Running coprocessor post-open hooks");
       coprocessorHost.postOpen();
     }
+
+    metricsTableRequests = new MetricsTableRequests(htableDescriptor.getTableName(), conf);
+
     status.markComplete("Region opened successfully");
     return nextSeqId;
   }
@@ -1875,6 +1889,13 @@ public Pair<byte[], Collection<HStoreFile>> call() throws IOException {
         writeRegionCloseMarker(wal);
       }
       this.closed.set(true);
+
+      // Decrease refCount of table latency metric registry.
+      // Do this after closed#set to make sure only -1.
+      if (metricsTableRequests != null) {
+        metricsTableRequests.removeRegistry();
+      }
+
       if (!canFlush) {
         decrMemStoreSize(this.memStoreSizing.getMemStoreSize());
       } else if (this.memStoreSizing.getDataSize() != 0) {
@@ -4691,8 +4712,7 @@ private OperationStatus[] batchMutate(BatchOperation<?> batchOp) throws IOExcept
       }
     } finally {
       if (rsServices != null && rsServices.getMetrics() != null) {
-        rsServices.getMetrics().updateWriteQueryMeter(this.htableDescriptor.getTableName(),
-          batchOp.size());
+        rsServices.getMetrics().updateWriteQueryMeter(this, batchOp.size());
       }
       batchOp.closeRegionOperation();
     }
@@ -7885,7 +7905,7 @@ void metricsUpdateForGet(List<Cell> results, long before) {
       this.metricsRegion.updateGet(EnvironmentEdgeManager.currentTime() - before);
     }
     if (this.rsServices != null && this.rsServices.getMetrics() != null) {
-      rsServices.getMetrics().updateReadQueryMeter(getRegionInfo().getTable(), 1);
+      rsServices.getMetrics().updateReadQueryMeter(this, 1);
     }
 
   }