[GitHub] [hbase] saintstack commented on a change in pull request #2483: HBASE-25026 - Create a metric to track scans that have no start row and/or stop row

[GitBox <gi...@apache.org>]

saintstack commented on a change in pull request #2483:
URL: https://github.com/apache/hbase/pull/2483#discussion_r497723563



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -1,4 +1,4 @@
-/**
+  /**

Review comment:
       No need of space here

##########
File path: hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java
##########
@@ -571,6 +571,9 @@
   String RPC_SCAN_REQUEST_COUNT = "rpcScanRequestCount";
   String RPC_SCAN_REQUEST_COUNT_DESC =
       "Number of rpc scan requests this RegionServer has answered.";
+  String RPC_FULL_SCAN_REQUEST_COUNT = "rpcFullScanRequestCount";
+  String RPC_FULL_SCAN_REQUEST_COUNT_DESC =
+      "Number of rpc scan requests that were possible full region scans.";

Review comment:
       Full Region or full Table?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -7030,6 +7030,7 @@ public String toString() {
     protected Cell joinedContinuationRow = null;
     private boolean filterClosed = false;
 
+    protected final byte[] startRow;

Review comment:
       Does this need javadoc/comment? It is NOT the same as the RegionInfo#startRow, right? If so, say so (my first though was why this data member when we have this info in RegionInfo... but this is the scan startRow...)

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -3498,6 +3501,18 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
       throw new ServiceException(e);
     }
     HRegion region = rsh.r;
+    // If the start row is the actual start Row of the region and end row is actual end row of region
+    // or
+    // If the stop is not specified or the stop Row is greater than the end key of the current region
+    if ((Bytes.equals(rsh.s.getStartRow(), region.getRegionInfo().getStartKey())
+      && Bytes.equals(rsh.s.getStopRow(), region.getRegionInfo().getEndKey()))
+      || Bytes.equals(rsh.s.getStopRow(), HConstants.EMPTY_END_ROW)
+      || (Bytes.compareTo(region.getRegionInfo().getEndKey(), rsh.s.getStopRow()) <= 0
+      && !Bytes.equals(region.getRegionInfo().getEndKey(), HConstants.EMPTY_END_ROW))) {
+      if(!region.getTableDescriptor().isMetaTable()) {

Review comment:
       Why exclude meta table scans? Do this check first before doing all the above byte array compares?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org