From 300b0a650f94567a1b0ca281cbc3d00f7eeb6956 Mon Sep 17 00:00:00 2001 From: gkanade Date: Mon, 16 Nov 2020 19:18:59 -0800 Subject: [PATCH] HBASE-25026 Create a metric to track full region scans RPCs Add new metric rpcFullScanRequestCount to track number of requests that are full region scans. Can be used to notify user to check if this is truly intended. Signed-off-by Anoop Sam John Signed-off-by Ramkrishna S Vasudevan --- .../MetricsRegionServerSource.java | 3 + .../MetricsRegionServerSourceImpl.java | 2 + .../MetricsRegionServerWrapper.java | 5 + .../hbase/regionserver/HRegionServer.java | 1 + .../MetricsRegionServerWrapperImpl.java | 5 + .../hbase/regionserver/RSRpcServices.java | 38 ++++- .../regionserver/RegionServerServices.java | 2 +- .../MetricsRegionServerWrapperStub.java | 5 + .../TestScannerRPCScanMetrics.java | 158 ++++++++++++++++++ 9 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScannerRPCScanMetrics.java diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java index afeccee504a..9d880d16137 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java @@ -571,6 +571,9 @@ public interface MetricsRegionServerSource extends BaseSource, JvmPauseMonitorSo 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."; String RPC_MULTI_REQUEST_COUNT = "rpcMultiRequestCount"; String RPC_MULTI_REQUEST_COUNT_DESC = "Number of rpc multi requests this RegionServer has answered."; diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java index 32a2b1e99f4..7b0225482cb 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java @@ -351,6 +351,8 @@ public class MetricsRegionServerSourceImpl rsWrap.getWriteRequestsCount()) .addCounter(Interns.info(RPC_GET_REQUEST_COUNT, RPC_GET_REQUEST_COUNT_DESC), rsWrap.getRpcGetRequestsCount()) + .addCounter(Interns.info(RPC_FULL_SCAN_REQUEST_COUNT, RPC_FULL_SCAN_REQUEST_COUNT_DESC), + rsWrap.getRpcFullScanRequestsCount()) .addCounter(Interns.info(RPC_SCAN_REQUEST_COUNT, RPC_SCAN_REQUEST_COUNT_DESC), rsWrap.getRpcScanRequestsCount()) .addCounter(Interns.info(RPC_MULTI_REQUEST_COUNT, RPC_MULTI_REQUEST_COUNT_DESC), diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapper.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapper.java index 4ca2d0e85ad..e4305330b67 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapper.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapper.java @@ -492,6 +492,11 @@ public interface MetricsRegionServerWrapper { */ long getRpcScanRequestsCount(); + /** + * Get the number of full region rpc scan requests to this region server. + */ + long getRpcFullScanRequestsCount(); + /** * Get the number of rpc multi requests to this region server. */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 45b8bbe53b3..74aec58ebba 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -2806,6 +2806,7 @@ public class HRegionServer extends Thread implements rpcServices.requestCount.reset(); rpcServices.rpcGetRequestCount.reset(); rpcServices.rpcScanRequestCount.reset(); + rpcServices.rpcFullScanRequestCount.reset(); rpcServices.rpcMultiRequestCount.reset(); rpcServices.rpcMutateRequestCount.reset(); LOG.info("reportForDuty to master=" + masterServerName + " with port=" diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java index 8ce2baaef4d..5a358bc0d8e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java @@ -498,6 +498,11 @@ class MetricsRegionServerWrapperImpl return regionServer.rpcServices.rpcScanRequestCount.sum(); } + @Override + public long getRpcFullScanRequestsCount() { + return regionServer.rpcServices.rpcFullScanRequestCount.sum(); + } + @Override public long getRpcMultiRequestsCount() { return regionServer.rpcServices.rpcMultiRequestCount.sum(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index ec280b8b01c..8aaddce7734 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -314,6 +314,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler, // Request counter for rpc scan final LongAdder rpcScanRequestCount = new LongAdder(); + + // Request counter for scans that might end up in full scans + final LongAdder rpcFullScanRequestCount = new LongAdder(); // Request counter for rpc multi final LongAdder rpcMultiRequestCount = new LongAdder(); @@ -459,15 +462,18 @@ public class RSRpcServices implements HBaseRPCErrorHandler, private final RpcCallback shippedCallback; private byte[] rowOfLastPartialResult; private boolean needCursor; + private boolean fullRegionScan; public RegionScannerHolder(String scannerName, RegionScanner s, HRegion r, - RpcCallback closeCallBack, RpcCallback shippedCallback, boolean needCursor) { + RpcCallback closeCallBack, RpcCallback shippedCallback, boolean needCursor, + boolean fullRegionScan) { this.scannerName = scannerName; this.s = s; this.r = r; this.closeCallBack = closeCallBack; this.shippedCallback = shippedCallback; this.needCursor = needCursor; + this.fullRegionScan = fullRegionScan; } public long getNextCallSeq() { @@ -1326,7 +1332,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } private RegionScannerHolder addScanner(String scannerName, RegionScanner s, Shipper shipper, - HRegion r, boolean needCursor) throws LeaseStillHeldException { + HRegion r, boolean needCursor, boolean fullRegionScan) throws LeaseStillHeldException { Lease lease = regionServer.getLeaseManager().createLease( scannerName, this.scannerLeaseTimeoutPeriod, new ScannerListener(scannerName)); RpcCallback shippedCallback = new RegionScannerShippedCallBack(scannerName, shipper, lease); @@ -1336,14 +1342,30 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } else { closeCallback = new RegionScannerCloseCallBack(s); } + RegionScannerHolder rsh = - new RegionScannerHolder(scannerName, s, r, closeCallback, shippedCallback, needCursor); + new RegionScannerHolder(scannerName, s, r, closeCallback, shippedCallback, + needCursor, fullRegionScan); RegionScannerHolder existing = scanners.putIfAbsent(scannerName, rsh); assert existing == null : "scannerId must be unique within regionserver's whole lifecycle! " + scannerName; return rsh; } + private boolean isFullRegionScan(Scan scan, HRegion region) { + // If the scan start row equals or less than the start key of the region + // and stop row greater than equals end key (if stop row present) + // or if the stop row is empty + // account this as a full region scan + if (Bytes.compareTo(scan.getStartRow(), region.getRegionInfo().getStartKey()) <= 0 + && (Bytes.compareTo(scan.getStopRow(), region.getRegionInfo().getEndKey()) >= 0 && + !Bytes.equals(region.getRegionInfo().getEndKey(), HConstants.EMPTY_END_ROW) + || Bytes.equals(scan.getStopRow(), HConstants.EMPTY_END_ROW))) { + return true; + } + return false; + } + /** * Find the HRegion based on a region specifier * @@ -3164,7 +3186,12 @@ public class RSRpcServices implements HBaseRPCErrorHandler, builder.setMvccReadPoint(scanner.getMvccReadPoint()); builder.setTtl(scannerLeaseTimeoutPeriod); String scannerName = String.valueOf(scannerId); - return addScanner(scannerName, scanner, shipper, region, scan.isNeedCursorResult()); + + boolean fullRegionScan = !region.getRegionInfo().getTable().isSystemTable() && + isFullRegionScan(scan, region); + + return addScanner(scannerName, scanner, shipper, region, scan.isNeedCursorResult(), + fullRegionScan); } private void checkScanNextCallSeq(ScanRequest request, RegionScannerHolder rsh) @@ -3478,6 +3505,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } throw new ServiceException(e); } + if (rsh.fullRegionScan) { + rpcFullScanRequestCount.increment(); + } HRegion region = rsh.r; String scannerName = rsh.scannerName; LeaseManager.Lease lease; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index 9e395d4f5cc..fce8df17264 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -317,4 +317,4 @@ public interface RegionServerServices extends Server, MutableOnlineRegions, Favo * @return {@link ZKPermissionWatcher} */ ZKPermissionWatcher getZKPermissionWatcher(); -} \ No newline at end of file +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperStub.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperStub.java index 0a0485c1aaf..ac361997bb1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperStub.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperStub.java @@ -629,4 +629,9 @@ public class MetricsRegionServerWrapperStub implements MetricsRegionServerWrappe public long getAverageRegionSize() { return 10000000; } + + @Override + public long getRpcFullScanRequestsCount() { + return 10; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScannerRPCScanMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScannerRPCScanMetrics.java new file mode 100644 index 00000000000..d9a9e251f13 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScannerRPCScanMetrics.java @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MiniHBaseCluster.MiniHBaseClusterRegionServer; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TestClientScannerRPCTimeout; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.io.IOException; + +@Category({ RegionServerTests.class, MediumTests.class}) +public class TestScannerRPCScanMetrics { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestScannerRPCScanMetrics.class); + + private static final Logger LOG = LoggerFactory.getLogger(TestScannerRPCScanMetrics.class); + private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static final byte[] FAMILY = Bytes.toBytes("testFamily"); + private static final byte[] QUALIFIER = Bytes.toBytes("testQualifier"); + private static final byte[] VALUE = Bytes.toBytes("testValue"); + + + @Rule + public TestName name = new TestName(); + + @BeforeClass + public static void setupBeforeClass() throws Exception { + Configuration conf = TEST_UTIL.getConfiguration(); + conf.setStrings(HConstants.REGION_SERVER_IMPL, RegionServerWithScanMetrics.class.getName()); + TEST_UTIL.startMiniCluster(1); + } + + @AfterClass + public static void tearDownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + @Test + public void testScannerRPCScanMetrics() throws Exception { + final TableName tableName = TableName.valueOf(name.getMethodName()); + byte[][] splits = new byte[1][]; + splits[0] = Bytes.toBytes("row-4"); + Table ht = TEST_UTIL.createTable(tableName, FAMILY,splits); + byte[] r0 = Bytes.toBytes("row-0"); + byte[] r1 = Bytes.toBytes("row-1"); + byte[] r2 = Bytes.toBytes("row-2"); + byte[] r3 = Bytes.toBytes("row-3"); + putToTable(ht, r0); + putToTable(ht, r1); + putToTable(ht, r2); + putToTable(ht, r3); + LOG.info("Wrote our four table entries"); + Scan scan1 = new Scan(); + scan1.withStartRow(r0); + // This scan should not increment rpc full scan count (start row specified) + scan1.withStopRow(Bytes.toBytes("row-4")); + scanNextIterate(ht, scan1); + Scan scan2 = new Scan(); + scan2.withStartRow(r1); + // This scan should increment rpc full scan count by 1 (for second region only) + scanNextIterate(ht, scan2); + Scan scan3 = new Scan(); + scan3.withStopRow(Bytes.toBytes("row-5")); + // This scan should increment rpc full scan count by 1 (for firts region only) + scanNextIterate(ht, scan3); + Scan scan4 = new Scan(); + scan4.withStartRow(r1); + scan4.withStopRow(r2); + // This scan should not increment rpc full scan count (both start and stop row) + scanNextIterate(ht, scan4); + Scan dummyScan = new Scan(); + // This scan should increment rpc full scan count by 2 (both regions - no stop/start row) + scanNextIterate(ht, dummyScan); + + RSRpcServices testClusterRSRPCServices = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0) + .rpcServices; + assertEquals(4, testClusterRSRPCServices.rpcFullScanRequestCount.intValue()); + } + + private void putToTable(Table ht, byte[] rowkey) throws IOException { + Put put = new Put(rowkey); + put.addColumn(FAMILY, QUALIFIER, VALUE); + ht.put(put); + } + + private void scanNextIterate(Table ht, Scan scan) throws Exception{ + ResultScanner scanner = ht.getScanner(scan); + for (Result result = scanner.next(); result != null; result = scanner.next()) + { + // Use the result object + } + scanner.close(); + } + + private static class RegionServerWithScanMetrics extends MiniHBaseClusterRegionServer { + public RegionServerWithScanMetrics(Configuration conf) + throws IOException, InterruptedException { + super(conf); + } + + protected RSRpcServices createRPCServices() throws IOException { + return new RSRPCServicesWithScanMetrics(this); + } + } + private static class RSRPCServicesWithScanMetrics extends RSRpcServices { + public long getScanRequestCount() { + return super.rpcScanRequestCount.longValue(); + } + public long getFullScanRequestCount() { + return super.rpcFullScanRequestCount.longValue(); + } + public RSRPCServicesWithScanMetrics(HRegionServer rs) + throws IOException { + super(rs); + } + } + +}