HBASE-15377 Per-RS Get metric is time based, per-region metric is size-based (Heng Chen)

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java

@@ -258,6 +258,7 @@ public interface MetricsRegionServerSource extends BaseSource {
   String UPDATES_BLOCKED_DESC =
       "Number of MS updates have been blocked so that the memstore can be flushed.";
   String DELETE_KEY = "delete";
+  String GET_SIZE_KEY = "getSize";
   String GET_KEY = "get";
   String INCREMENT_KEY = "increment";
   String MUTATE_KEY = "mutate";

hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java

@@ -59,7 +59,13 @@ public interface MetricsRegionSource extends Comparable<MetricsRegionSource> {
    * Update count and sizes of gets.
    * @param getSize size in bytes of the resulting key values for a get
    */
-  void updateGet(long getSize);
+  void updateGetSize(long getSize);
+
+  /**
+   * Update time of gets
+   * @param mills time for this get operation.
+   */
+  void updateGet(long mills);
 
   /**
    * Update the count and sizes of resultScanner.next()

hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java

@@ -48,6 +48,7 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
   private final String regionNamePrefix;
   private final String regionPutKey;
   private final String regionDeleteKey;
+  private final String regionGetSizeKey;
   private final String regionGetKey;
   private final String regionIncrementKey;
   private final String regionAppendKey;
@@ -58,6 +59,7 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
   private final MutableFastCounter regionDelete;
   private final MutableFastCounter regionIncrement;
   private final MutableFastCounter regionAppend;
+  private final MetricHistogram regionGetSize;
   private final MetricHistogram regionGet;
   private final MetricHistogram regionScanSize;
   private final MetricHistogram regionScanTime;
@@ -93,6 +95,9 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
     regionAppendKey = regionNamePrefix + MetricsRegionServerSource.APPEND_KEY + suffix;
     regionAppend = registry.getCounter(regionAppendKey, 0L);
 
+    regionGetSizeKey = regionNamePrefix + MetricsRegionServerSource.GET_SIZE_KEY;
+    regionGetSize = registry.newSizeHistogram(regionGetSizeKey);
+
     regionGetKey = regionNamePrefix + MetricsRegionServerSource.GET_KEY;
     regionGet = registry.newTimeHistogram(regionGetKey);
 
@@ -129,9 +134,11 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
       registry.removeMetric(regionDeleteKey);
       registry.removeMetric(regionIncrementKey);
       registry.removeMetric(regionAppendKey);
+      registry.removeMetric(regionGetSizeKey);
       registry.removeMetric(regionGetKey);
       registry.removeMetric(regionScanSizeKey);
       registry.removeMetric(regionScanTimeKey);
+      registry.removeHistogramMetrics(regionGetSizeKey);
       registry.removeHistogramMetrics(regionGetKey);
       registry.removeHistogramMetrics(regionScanSizeKey);
       registry.removeHistogramMetrics(regionScanTimeKey);
@@ -151,8 +158,13 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
   }
 
   @Override
-  public void updateGet(long getSize) {
+  public void updateGetSize(long getSize) {
-    regionGet.add(getSize);
+    regionGetSize.add(getSize);
+  }
+
+  @Override
+  public void updateGet(long mills) {
+    regionGet.add(mills);
   }
 
   @Override

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

@@ -6702,7 +6702,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
         return results;
       }
     }
-
+    long before =  EnvironmentEdgeManager.currentTime();
     Scan scan = new Scan(get);
 
     RegionScanner scanner = null;
@@ -6719,12 +6719,12 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
       coprocessorHost.postGet(get, results);
     }
 
-    metricsUpdateForGet(results);
+    metricsUpdateForGet(results, before);
 
     return results;
   }
 
-  void metricsUpdateForGet(List<Cell> results) {
+  void metricsUpdateForGet(List<Cell> results, long before) {
     if (this.metricsRegion != null) {
       long totalSize = 0L;
       for (Cell cell : results) {
@@ -6732,7 +6732,8 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
         // to know the serialization of how the codec works with it??
         totalSize += CellUtil.estimatedSerializedSizeOf(cell);
       }
-      this.metricsRegion.updateGet(totalSize);
+      this.metricsRegion.updateGetSize(totalSize);
+      this.metricsRegion.updateGet(EnvironmentEdgeManager.currentTime() - before);
     }
   }
 

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

@@ -49,8 +49,12 @@ public class MetricsRegion {
     source.updateDelete();
   }
 
-  public void updateGet(final long getSize) {
+  public void updateGetSize(final long getSize) {
-    source.updateGet(getSize);
+    source.updateGetSize(getSize);
+  }
+
+  public void updateGet(final long t) {
+    source.updateGet(t);
   }
 
   public void updateScanSize(final long scanSize) {

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

@@ -2197,7 +2197,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
             .create(results, get.isCheckExistenceOnly() ? !results.isEmpty() : null, stale);
       }
     }
-
+    long before = EnvironmentEdgeManager.currentTime();
     Scan scan = new Scan(get);
 
     RegionScanner scanner = null;
@@ -2227,7 +2227,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     if (region.getCoprocessorHost() != null) {
       region.getCoprocessorHost().postGet(get, results);
     }
-    region.metricsUpdateForGet(results);
+    region.metricsUpdateForGet(results, before);
     return Result.create(results, get.isCheckExistenceOnly() ? !results.isEmpty() : null, stale);
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java

@@ -187,6 +187,52 @@ public class TestRegionServerMetrics {
     table.close();
   }
 
+  @Test
+  public void testGet() throws Exception {
+    String tableNameString = "testGet";
+    TableName tName = TableName.valueOf(tableNameString);
+    byte[] cfName = Bytes.toBytes("d");
+    byte[] row = Bytes.toBytes("rk");
+    byte[] qualifier = Bytes.toBytes("qual");
+    byte[] initValue = Bytes.toBytes("Value");
+
+    TEST_UTIL.createTable(tName, cfName);
+
+    Connection connection = TEST_UTIL.getConnection();
+    connection.getTable(tName).close(); //wait for the table to come up.
+
+    // Do a first put to be sure that the connection is established, meta is there and so on.
+    Table table = connection.getTable(tName);
+    Put p = new Put(row);
+    p.addColumn(cfName, qualifier, initValue);
+    table.put(p);
+
+    Get g = new Get(row);
+    for (int i=0; i< 10; i++) {
+      table.get(g);
+    }
+
+    metricsRegionServer.getRegionServerWrapper().forceRecompute();
+
+    try (RegionLocator locator = connection.getRegionLocator(tName)) {
+      for ( HRegionLocation location: locator.getAllRegionLocations()) {
+        HRegionInfo i = location.getRegionInfo();
+        MetricsRegionAggregateSource agg = rs.getRegion(i.getRegionName())
+          .getMetrics()
+          .getSource()
+          .getAggregateSource();
+        String prefix = "namespace_"+NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR+
+          "_table_"+tableNameString +
+          "_region_" + i.getEncodedName()+
+          "_metric";
+        metricsHelper.assertCounter(prefix + "_getSizeNumOps", 10, agg);
+        metricsHelper.assertCounter(prefix + "_getNumOps", 10, agg);
+      }
+      metricsHelper.assertCounterGt("Get_num_ops", 10, serverSource);
+    }
+    table.close();
+  }
+
   @Test
   public void testMutationsWithoutWal() throws Exception {
     TableName tableName = TableName.valueOf("testMutationsWithoutWal");