HBASE-24205 - Avoid metric collection for flushes and compactions (#1918)

* HBASE-24205 - Avoid metric collection for flushes and compactions

* Use the existing matcher.isUserScan() to decide the scan type

* Added a comment saying when we track the metrics

Author: ramkrish86

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

@@ -342,13 +342,30 @@ public StoreScanner(ScanInfo scanInfo, ScanType scanType,
     seekAllScanner(scanInfo, scanners);
   }
 
+  // Used to instantiate a scanner for user scan in test
+  @VisibleForTesting
+  StoreScanner(Scan scan, ScanInfo scanInfo, NavigableSet<byte[]> columns,
+      List<? extends KeyValueScanner> scanners, ScanType scanType) throws IOException {
+    // 0 is passed as readpoint because the test bypasses Store
+    this(null, scan, scanInfo, columns != null ? columns.size() : 0, 0L, scan.getCacheBlocks(),
+        scanType);
+    if (scanType == ScanType.USER_SCAN) {
+      this.matcher =
+          UserScanQueryMatcher.create(scan, scanInfo, columns, oldestUnexpiredTS, now, null);
+    } else {
+      this.matcher = CompactionScanQueryMatcher.create(scanInfo, scanType, Long.MAX_VALUE,
+        HConstants.OLDEST_TIMESTAMP, oldestUnexpiredTS, now, null, null, null);
+    }
+    seekAllScanner(scanInfo, scanners);
+  }
+
   // Used to instantiate a scanner for user scan in test
   @VisibleForTesting
   StoreScanner(Scan scan, ScanInfo scanInfo, NavigableSet<byte[]> columns,
       List<? extends KeyValueScanner> scanners) throws IOException {
     // 0 is passed as readpoint because the test bypasses Store
-    this(null, scan, scanInfo, columns != null ? columns.size() : 0, 0L,
-        scan.getCacheBlocks(), ScanType.USER_SCAN);
+    this(null, scan, scanInfo, columns != null ? columns.size() : 0, 0L, scan.getCacheBlocks(),
+        ScanType.USER_SCAN);
     this.matcher =
         UserScanQueryMatcher.create(scan, scanInfo, columns, oldestUnexpiredTS, now, null);
     seekAllScanner(scanInfo, scanners);
@@ -569,7 +586,8 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
 
     int count = 0;
     long totalBytesRead = 0;
-    boolean onlyFromMemstore = true;
+    // track the cells for metrics only if it is a user read request.
+    boolean onlyFromMemstore = matcher.isUserScan();
     try {
       LOOP: do {
         // Update and check the time limit based on the configured value of cellsPerTimeoutCheck
@@ -747,7 +765,7 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
       return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
     } finally {
       // increment only if we have some result
-      if (count > 0) {
+      if (count > 0 && matcher.isUserScan()) {
         // if true increment memstore metrics, if not the mixed one
         updateMetricsStore(onlyFromMemstore);
       }

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

@@ -588,6 +588,27 @@ public void testScanSameTimestamp() throws IOException {
     }
   }
 
+  @Test
+  public void testNonUserScan() throws IOException {
+    // returns only 1 of these 2 even though same timestamp
+    KeyValue[] kvs = new KeyValue[] { create("R1", "cf", "a", 1, KeyValue.Type.Put, "dont-care"),
+        create("R1", "cf", "a", 1, KeyValue.Type.Put, "dont-care"), };
+    List<KeyValueScanner> scanners = Arrays.asList(
+      new KeyValueScanner[] { new KeyValueScanFixture(CellComparator.getInstance(), kvs) });
+
+    Scan scanSpec = new Scan().withStartRow(Bytes.toBytes("R1"));
+    // this only uses maxVersions (default=1) and TimeRange (default=all)
+    try (StoreScanner scan =
+        new StoreScanner(scanSpec, scanInfo, null, scanners, ScanType.COMPACT_RETAIN_DELETES)) {
+      List<Cell> results = new ArrayList<>();
+      assertEquals(true, scan.next(results));
+      assertEquals(1, results.size());
+      // the type is not a user scan. so it won't account for the memstore reads
+      assertEquals(0, scan.memstoreOnlyReads);
+      assertEquals(kvs[0], results.get(0));
+    }
+  }
+
   /*
    * Test test shows exactly how the matcher's return codes confuses the StoreScanner
    * and prevent it from doing the right thing.  Seeking once, then nexting twice