HBASE-25677 Server+table counters on each scan #nextRaw invocation becomes a bottleneck when heavy load (#3066)

Signed-off-by: stack <stack@apache.org>
Signed-off-by: Reid Chan <reidchan@apache.org>

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

@@ -6402,9 +6402,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
       if (!outResults.isEmpty()) {
         readRequestsCount.increment();
       }
-      if (rsServices != null && rsServices.getMetrics() != null) {
-        rsServices.getMetrics().updateReadQueryMeter(getRegionInfo().getTable());
-      }
 
       // If the size limit was reached it means a partial Result is being returned. Returning a
       // partial Result means that we should not reset the filters; filters should only be reset in

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

@@ -28,12 +28,11 @@ import org.apache.hadoop.hbase.metrics.Timer;
 import org.apache.hadoop.conf.Configuration;
 
 /**
- * <p>
+ * Maintains regionserver statistics and publishes them through the metrics interfaces.
- * This class is for maintaining the various regionserver statistics
- * and publishing them through the metrics interfaces.
- * </p>
  * This class has a number of metrics variables that are publicly accessible;
- * these variables (objects) have methods to update their values.
+ * these variables (objects) have methods to update their values. Batch your updates rather than
+ * call on each instance else all threads will do nothing but contend trying to maintain metric
+ * counters!
  */
 @InterfaceStability.Evolving
 @InterfaceAudience.Private
@@ -49,7 +48,9 @@ public class MetricsRegionServer {
 
   private MetricRegistry metricRegistry;
   private Timer bulkLoadTimer;
+  // Incremented once for each call to Scan#nextRaw
   private Meter serverReadQueryMeter;
+  // Incremented per write.
   private Meter serverWriteQueryMeter;
 
   public MetricsRegionServer(MetricsRegionServerWrapper regionServerWrapper, Configuration conf) {
@@ -241,13 +242,6 @@ public class MetricsRegionServer {
     this.serverReadQueryMeter.mark(count);
   }
 
-  public void updateReadQueryMeter(TableName tn) {
-    if (tableMetrics != null && tn != null) {
-      tableMetrics.updateTableReadQueryMeter(tn);
-    }
-    this.serverReadQueryMeter.mark();
-  }
-
   public void updateWriteQueryMeter(TableName tn, long count) {
     if (tableMetrics != null && tn != null) {
       tableMetrics.updateTableWriteQueryMeter(tn, count);

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

@@ -2901,10 +2901,13 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     // arbitrary 32. TODO: keep record of general size of results being returned.
     List<Cell> values = new ArrayList<Cell>(32);
     region.startRegionOperation(Operation.SCAN);
+    long before = EnvironmentEdgeManager.currentTime();
+    // Used to check if we've matched the row limit set on the Scan
+    int numOfCompleteRows = 0;
+    // Count of times we call nextRaw; can be > numOfCompleteRows.
+    int numOfNextRawCalls = 0;
     try {
       int numOfResults = 0;
-      int numOfCompleteRows = 0;
-      long before = EnvironmentEdgeManager.currentTime();
       synchronized (scanner) {
         boolean stale = (region.getRegionInfo().getReplicaId() != 0);
         boolean clientHandlesPartials =
@@ -2958,6 +2961,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
 
           // Collect values to be returned here
           moreRows = scanner.nextRaw(values, scannerContext);
+          numOfNextRawCalls++;
 
           if (!values.isEmpty()) {
             if (limitOfRows > 0) {
@@ -3053,6 +3057,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
           builder.setScanMetrics(metricBuilder.build());
         }
       }
+    } finally {
+      region.closeRegionOperation();
+      // Update serverside metrics, even on error.
       long end = EnvironmentEdgeManager.currentTime();
       long responseCellSize = context != null ? context.getResponseCellSize() : 0;
       region.getMetrics().updateScanTime(end - before);
@@ -3061,9 +3068,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
             region.getTableDesc().getTableName(), responseCellSize);
         regionServer.metricsRegionServer.updateScanTime(
             region.getTableDesc().getTableName(), end - before);
+        regionServer.metricsRegionServer
+          .updateReadQueryMeter(region.getRegionInfo().getTable(), numOfNextRawCalls);
       }
-    } finally {
-      region.closeRegionOperation();
     }
     // coprocessor postNext hook
     if (region.getCoprocessorHost() != null) {