HBASE-15376 ScanNext metric is size-based while every other per-operation metric is time based

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

@@ -92,7 +92,12 @@ public interface MetricsRegionServerSource extends BaseSource {
    *
    * @param scanSize size of the scan
    */
-  void updateScannerNext(long scanSize);
+  void updateScanSize(long scanSize);
+
+  /**
+   * Update the scan time.
+   * */
+  void updateScanTime(long t);
 
   /**
    * Increment the number of slow Puts that have happened.
@@ -258,7 +263,9 @@ public interface MetricsRegionServerSource extends BaseSource {
   String MUTATE_KEY = "mutate";
   String APPEND_KEY = "append";
   String REPLAY_KEY = "replay";
-  String SCAN_NEXT_KEY = "scanNext";
+  String SCAN_SIZE_KEY = "scanSize";
+  String SCAN_TIME_KEY = "scanTime";
+
   String SLOW_MUTATE_KEY = "slowPutCount";
   String SLOW_GET_KEY = "slowGetCount";
   String SLOW_DELETE_KEY = "slowDeleteCount";

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

@@ -65,7 +65,13 @@ public interface MetricsRegionSource extends Comparable<MetricsRegionSource> {
    * Update the count and sizes of resultScanner.next()
    * @param scanSize Size in bytes of the resulting key values for a next()
    */
-  void updateScan(long scanSize);
+  void updateScanSize(long scanSize);
+
+  /**
+   * Update time used of resultScanner.next().
+   * */
+  void updateScanTime(long mills);
+
   /**
    * Update related counts of increments.
    */

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

@@ -43,7 +43,8 @@ public class MetricsRegionServerSourceImpl
   private final MetricHistogram incrementHisto;
   private final MetricHistogram appendHisto;
   private final MetricHistogram replayHisto;
-  private final MetricHistogram scanNextHisto;
+  private final MetricHistogram scanSizeHisto;
+  private final MetricHistogram scanTimeHisto;
 
   private final MutableFastCounter slowPut;
   private final MutableFastCounter slowDelete;
@@ -84,7 +85,8 @@ public class MetricsRegionServerSourceImpl
     slowAppend = getMetricsRegistry().newCounter(SLOW_APPEND_KEY, SLOW_APPEND_DESC, 0L);
     
     replayHisto = getMetricsRegistry().newTimeHistogram(REPLAY_KEY);
-    scanNextHisto = getMetricsRegistry().newTimeHistogram(SCAN_NEXT_KEY);
+    scanSizeHisto = getMetricsRegistry().newSizeHistogram(SCAN_SIZE_KEY);
+    scanTimeHisto = getMetricsRegistry().newTimeHistogram(SCAN_TIME_KEY);
 
     splitTimeHisto = getMetricsRegistry().newTimeHistogram(SPLIT_KEY);
     flushTimeHisto = getMetricsRegistry().newTimeHistogram(FLUSH_KEY);
@@ -124,8 +126,13 @@ public class MetricsRegionServerSourceImpl
   }
 
   @Override
-  public void updateScannerNext(long scanSize) {
+  public void updateScanSize(long scanSize) {
-    scanNextHisto.add(scanSize);
+    scanSizeHisto.add(scanSize);
+  }
+
+  @Override
+  public void updateScanTime(long t) {
+    scanTimeHisto.add(t);
   }
 
   @Override

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

@@ -51,14 +51,16 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
   private final String regionGetKey;
   private final String regionIncrementKey;
   private final String regionAppendKey;
-  private final String regionScanNextKey;
+  private final String regionScanSizeKey;
+  private final String regionScanTimeKey;
 
   private final MutableFastCounter regionPut;
   private final MutableFastCounter regionDelete;
   private final MutableFastCounter regionIncrement;
   private final MutableFastCounter regionAppend;
   private final MetricHistogram regionGet;
-  private final MetricHistogram regionScanNext;
+  private final MetricHistogram regionScanSize;
+  private final MetricHistogram regionScanTime;
   private final int hashCode;
 
   public MetricsRegionSourceImpl(MetricsRegionWrapper regionWrapper,
@@ -94,8 +96,11 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
     regionGetKey = regionNamePrefix + MetricsRegionServerSource.GET_KEY;
     regionGet = registry.newTimeHistogram(regionGetKey);
 
-    regionScanNextKey = regionNamePrefix + MetricsRegionServerSource.SCAN_NEXT_KEY;
+    regionScanSizeKey = regionNamePrefix + MetricsRegionServerSource.SCAN_SIZE_KEY;
-    regionScanNext = registry.newTimeHistogram(regionScanNextKey);
+    regionScanSize = registry.newSizeHistogram(regionScanSizeKey);
+
+    regionScanTimeKey = regionNamePrefix + MetricsRegionServerSource.SCAN_TIME_KEY;
+    regionScanTime = registry.newTimeHistogram(regionScanTimeKey);
 
     hashCode = regionWrapper.getRegionHashCode();
   }
@@ -125,9 +130,11 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
       registry.removeMetric(regionIncrementKey);
       registry.removeMetric(regionAppendKey);
       registry.removeMetric(regionGetKey);
-      registry.removeMetric(regionScanNextKey);
+      registry.removeMetric(regionScanSizeKey);
+      registry.removeMetric(regionScanTimeKey);
       registry.removeHistogramMetrics(regionGetKey);
-      registry.removeHistogramMetrics(regionScanNextKey);
+      registry.removeHistogramMetrics(regionScanSizeKey);
+      registry.removeHistogramMetrics(regionScanTimeKey);
 
       regionWrapper = null;
     }
@@ -149,8 +156,13 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
   }
 
   @Override
-  public void updateScan(long scanSize) {
+  public void updateScanSize(long scanSize) {
-    regionScanNext.add(scanSize);
+    regionScanSize.add(scanSize);
+  }
+
+  @Override
+  public void updateScanTime(long mills) {
+    regionScanTime.add(mills);
   }
 
   @Override

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

@@ -53,8 +53,12 @@ public class MetricsRegion {
     source.updateGet(getSize);
   }
 
-  public void updateScanNext(final long scanSize) {
+  public void updateScanSize(final long scanSize) {
-    source.updateScan(scanSize);
+    source.updateScanSize(scanSize);
+  }
+
+  public void updateScanTime(final long t) {
+    source.updateScanTime(t);
   }
 
   public void updateAppend() {

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

@@ -97,8 +97,12 @@ public class MetricsRegionServer {
     serverSource.updateReplay(t);
   }
 
-  public void updateScannerNext(long scanSize){
+  public void updateScanSize(long scanSize){
-    serverSource.updateScannerNext(scanSize);
+    serverSource.updateScanSize(scanSize);
+  }
+
+  public void updateScanTime(long t) {
+    serverSource.updateScanTime(t);
   }
 
   public void updateSplitTime(long t) {

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

@@ -2682,6 +2682,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
             region.startRegionOperation(Operation.SCAN);
             try {
               int i = 0;
+              long before = EnvironmentEdgeManager.currentTime();
               synchronized(scanner) {
                 boolean stale = (region.getRegionInfo().getReplicaId() != 0);
                 boolean clientHandlesPartials =
@@ -2814,10 +2815,13 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
                 }
               }
               region.updateReadRequestsCount(i);
+              long end = EnvironmentEdgeManager.currentTime();
               long responseCellSize = context != null ? context.getResponseCellSize() : 0;
-              region.getMetrics().updateScanNext(responseCellSize);
+              region.getMetrics().updateScanSize(responseCellSize);
+              region.getMetrics().updateScanTime(end - before);
               if (regionServer.metricsRegionServer != null) {
-                regionServer.metricsRegionServer.updateScannerNext(responseCellSize);
+                regionServer.metricsRegionServer.updateScanSize(responseCellSize);
+                regionServer.metricsRegionServer.updateScanTime(end - before);
               }
             } finally {
               region.closeRegionOperation();

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

@@ -348,8 +348,8 @@ public class TestRegionServerMetrics {
   }
 
   @Test
-  public void testScanNext() throws IOException {
+  public void testScanSize() throws IOException {
-    String tableNameString = "testScanNext";
+    String tableNameString = "testScanSize";
     TableName tableName = TableName.valueOf(tableNameString);
     byte[] cf = Bytes.toBytes("d");
     byte[] qualifier = Bytes.toBytes("qual");
@@ -387,9 +387,9 @@ public class TestRegionServerMetrics {
             "_table_"+tableNameString +
             "_region_" + i.getEncodedName()+
             "_metric";
-        metricsHelper.assertCounter(prefix + "_scanNextNumOps", NUM_SCAN_NEXT, agg);
+        metricsHelper.assertCounter(prefix + "_scanSizeNumOps", NUM_SCAN_NEXT, agg);
       }
-      metricsHelper.assertCounter("ScanNext_num_ops", numScanNext, serverSource);
+      metricsHelper.assertCounter("ScanSize_num_ops", numScanNext, serverSource);
     }
     try (Admin admin = TEST_UTIL.getHBaseAdmin()) {
       admin.disableTable(tableName);
@@ -398,8 +398,58 @@ public class TestRegionServerMetrics {
   }
 
   @Test
-  public void testScanNextForSmallScan() throws IOException {
+  public void testScanTime() throws IOException {
-    String tableNameString = "testScanNextSmall";
+    String tableNameString = "testScanTime";
+    TableName tableName = TableName.valueOf(tableNameString);
+    byte[] cf = Bytes.toBytes("d");
+    byte[] qualifier = Bytes.toBytes("qual");
+    byte[] val = Bytes.toBytes("One");
+
+    List<Put> puts = new ArrayList<>();
+    for (int insertCount =0; insertCount < 100; insertCount++) {
+      Put p = new Put(Bytes.toBytes("" + insertCount + "row"));
+      p.addColumn(cf, qualifier, val);
+      puts.add(p);
+    }
+    try (Table t = TEST_UTIL.createTable(tableName, cf)) {
+      t.put(puts);
+
+      Scan s = new Scan();
+      s.setBatch(1);
+      s.setCaching(1);
+      ResultScanner resultScanners = t.getScanner(s);
+
+      for (int nextCount = 0; nextCount < NUM_SCAN_NEXT; nextCount++) {
+        Result result = resultScanners.next();
+        assertNotNull(result);
+        assertEquals(1, result.size());
+      }
+    }
+    numScanNext += NUM_SCAN_NEXT;
+    try (RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName)) {
+      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 + "_scanTimeNumOps", NUM_SCAN_NEXT, agg);
+      }
+      metricsHelper.assertCounter("ScanTime_num_ops", numScanNext, serverSource);
+    }
+    try (Admin admin = TEST_UTIL.getHBaseAdmin()) {
+      admin.disableTable(tableName);
+      admin.deleteTable(tableName);
+    }
+  }
+
+  @Test
+  public void testScanSizeForSmallScan() throws IOException {
+    String tableNameString = "testScanSizeSmall";
     TableName tableName = TableName.valueOf(tableNameString);
     byte[] cf = Bytes.toBytes("d");
     byte[] qualifier = Bytes.toBytes("qual");
@@ -437,9 +487,9 @@ public class TestRegionServerMetrics {
             "_table_"+tableNameString +
             "_region_" + i.getEncodedName()+
             "_metric";
-        metricsHelper.assertCounter(prefix + "_scanNextNumOps", NUM_SCAN_NEXT, agg);
+        metricsHelper.assertCounter(prefix + "_scanSizeNumOps", NUM_SCAN_NEXT, agg);
       }
-      metricsHelper.assertCounter("ScanNext_num_ops", numScanNext, serverSource);
+      metricsHelper.assertCounter("ScanSize_num_ops", numScanNext, serverSource);
     }
     try (Admin admin = TEST_UTIL.getHBaseAdmin()) {
       admin.disableTable(tableName);