This is an automated email from the ASF dual-hosted git repository. ramkrishna pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push: new b17ba7b HBASE-24205 - Avoid metric collection for flushes and compactions (#1918) b17ba7b is described below commit b17ba7b81de8ae58857271a4eb7a4a49693341a1 Author: ramkrish86 <ram_krish...@hotmail.com> AuthorDate: Wed Jun 17 16:47:21 2020 +0530 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 --- .../hadoop/hbase/regionserver/StoreScanner.java | 26 ++++++++++++++++++---- .../hbase/regionserver/TestStoreScanner.java | 21 +++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java index 021d09b..b7c6b1c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java @@ -345,10 +345,27 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner // 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 class StoreScanner extends NonReversedNonLazyKeyValueScanner 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 class StoreScanner extends NonReversedNonLazyKeyValueScanner 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); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java index 885e7d2..ec71545 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java @@ -588,6 +588,27 @@ public class TestStoreScanner { } } + @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