From 39ad07026e2c740921a483d14e635454468e1831 Mon Sep 17 00:00:00 2001 From: Sergey Shelukhin Date: Tue, 30 Apr 2019 19:23:30 -0700 Subject: [PATCH 1/2] HBASE-22346 scanner priorities/deadline units are invalid for non-huge scanners --- .../hbase/master/MasterRpcServices.java | 5 + .../AnnotationReadingPriorityFunction.java | 96 +++++++++++-- .../hbase/regionserver/RSRpcServices.java | 33 ++++- .../hbase/regionserver/TestPriorityRpc.java | 131 +++++++++++++++++- 4 files changed, 246 insertions(+), 19 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index cd017d841b1f..611ac9a29c13 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -2862,4 +2862,9 @@ private boolean shouldSubmitSCP(ServerName serverName) { } return true; } + + @Override + public boolean isMaster() { + return true; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java index 2dbc1066be74..65a9d512f1ef 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java @@ -17,12 +17,15 @@ */ package org.apache.hadoop.hbase.regionserver; +import java.io.IOException; import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +43,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.RequestHeader; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.hbase.thirdparty.com.google.protobuf.ByteString; import org.apache.hbase.thirdparty.com.google.protobuf.Message; import org.apache.hbase.thirdparty.com.google.protobuf.TextFormat; import org.apache.hadoop.hbase.security.User; @@ -65,6 +69,7 @@ //All the argument classes declare a 'getRegion' method that returns a //RegionSpecifier object. Methods can be invoked on the returned object //to figure out whether it is a meta region or not. +// TODO: split the priority and deadline parts; they are currently completely unrelated @InterfaceAudience.Private public class AnnotationReadingPriorityFunction implements PriorityFunction { private static final Logger LOG = @@ -73,6 +78,18 @@ public class AnnotationReadingPriorityFunction implements PriorityFunction { /** Used to control the scan delay, currently sqrt(numNextCall * weight) */ public static final String SCAN_VTIME_WEIGHT_CONF_KEY = "hbase.ipc.server.scan.vtime.weight"; + /** When to use the actual time-based deadline for scanners */ + public static final String SCAN_DEADLINE_PRIORITY = "hbase.ipc.server.scan.deadline.only"; + private static final ScanDeadlineOnly SCAN_DEADLINE_PRIORITY_DEFAULT = ScanDeadlineOnly.NEVER; + enum ScanDeadlineOnly { + ALWAYS, + META, + NEVER + } + + private static final ByteString META_PREFIX = ByteString.copyFrom( + TableName.META_TABLE_NAME.toBytes()); + protected final Map annotatedQos; //We need to mock the regionserver instance for some unit tests (set via //setRegionServer method. @@ -94,6 +111,7 @@ public class AnnotationReadingPriorityFunction implements PriorityFunction { private final Map, Method>> methodMap = new HashMap<>(); private final float scanVirtualTimeWeight; + private final ScanDeadlineOnly scanDeadlineOnly; /** * Calls {@link #AnnotationReadingPriorityFunction(RSRpcServices, Class)} using the result of @@ -148,6 +166,26 @@ public AnnotationReadingPriorityFunction(final RSRpcServices rpcServices, Configuration conf = rpcServices.getConfiguration(); scanVirtualTimeWeight = conf.getFloat(SCAN_VTIME_WEIGHT_CONF_KEY, 1.0f); + scanDeadlineOnly = getScanDeadlineOnlyConf(rpcServices, conf); + } + + private static ScanDeadlineOnly getScanDeadlineOnlyConf( + RSRpcServices rpcServices, Configuration conf) { + ScanDeadlineOnly result = SCAN_DEADLINE_PRIORITY_DEFAULT; + String val = conf.get(SCAN_DEADLINE_PRIORITY, SCAN_DEADLINE_PRIORITY_DEFAULT.name()); + try { + result = ScanDeadlineOnly.valueOf(val.toUpperCase()); + } catch (IllegalArgumentException ex) { + LOG.warn("Invalid value for {} ({}); using the default", SCAN_DEADLINE_PRIORITY, val); + } + if (result == ScanDeadlineOnly.META && LoadBalancer.isTablesOnMaster(conf) + && LoadBalancer.isSystemTablesOnlyOnMaster(conf) && rpcServices.isMaster()) { + result = ScanDeadlineOnly.ALWAYS; + } + if (result != ScanDeadlineOnly.NEVER) { + LOG.info("Using deadline-based scanner priority {}", result); + } + return result; } private String capitalize(final String s) { @@ -262,21 +300,63 @@ protected int getBasePriority(RequestHeader header, Message param) { public long getDeadline(RequestHeader header, Message param) { if (param instanceof ScanRequest) { ScanRequest request = (ScanRequest)param; - if (!request.hasScannerId()) { - return 0; + boolean useDeadline; + long addToVTime = 0; + switch (scanDeadlineOnly) { + case ALWAYS: useDeadline = true; break; + case NEVER: useDeadline = false; break; + case META: { + useDeadline = isMetaScan(request); + // Make sure non-meta scans are generally after meta scans; add the default scanner delay. + if (!useDeadline) { + addToVTime = rpcServices.getScannerExpirationDelayMs(null); + } + break; + } + default: throw new AssertionError(scanDeadlineOnly); + } + if (useDeadline) { + return rpcServices.getScannerExpirationDelayMs( + request.hasScannerId() ? request.getScannerId() : null); + } else if (!request.hasScannerId()) { + return addToVTime; + } else { + // get the 'virtual time' of the scanner, and applies sqrt() to get a + // nice curve for the delay. More a scanner is used the less priority it gets. + // The weight is used to have more control on the delay. + long vtime = rpcServices.getScannerVirtualTime(request.getScannerId()); + return addToVTime + Math.round(Math.sqrt(vtime * scanVirtualTimeWeight)); } - - // get the 'virtual time' of the scanner, and applies sqrt() to get a - // nice curve for the delay. More a scanner is used the less priority it gets. - // The weight is used to have more control on the delay. - long vtime = rpcServices.getScannerVirtualTime(request.getScannerId()); - return Math.round(Math.sqrt(vtime * scanVirtualTimeWeight)); } return 0; } + private boolean isMetaScan(ScanRequest request) { + if (request.hasScannerId()) { + RegionScanner rs = rpcServices.getScanner(request.getScannerId()); + return (rs != null) && rs.getRegionInfo().isMetaRegion(); + } else if (request.hasRegion()) { + RegionSpecifier r = request.getRegion(); + if (r.hasType() && r.getType() == RegionSpecifier.RegionSpecifierType.REGION_NAME) { + return r.getValue().startsWith(META_PREFIX); // Common case shortcut. + } else { + try { + return rpcServices.getRegion(r).getTableDescriptor().isMetaRegion(); + } catch (IOException e) { + // Ignore NotServing; let's allow the scan to fail later, for consistency. + } + } + } + return false; + } + @VisibleForTesting void setRegionServer(final HRegionServer hrs) { this.rpcServices = hrs.getRSRpcServices(); } + + @VisibleForTesting + ScanDeadlineOnly getScanDeadlineOnly() { + return scanDeadlineOnly; + } } 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 688c03d678f1..30074c545325 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 @@ -437,17 +437,23 @@ private static final class RegionScannerHolder { private final HRegion r; private final RpcCallback closeCallBack; private final RpcCallback shippedCallback; + private final Lease lease; private byte[] rowOfLastPartialResult; private boolean needCursor; public RegionScannerHolder(String scannerName, RegionScanner s, HRegion r, - RpcCallback closeCallBack, RpcCallback shippedCallback, boolean needCursor) { + RpcCallback closeCallBack, RpcCallback shippedCallback, boolean needCursor, Lease lease) { this.scannerName = scannerName; this.s = s; this.r = r; this.closeCallBack = closeCallBack; this.shippedCallback = shippedCallback; this.needCursor = needCursor; + this.lease = lease; + } + + public long getExpirationDelayMs() { + return lease == null ? 0 : lease.getDelay(TimeUnit.MILLISECONDS); } public long getNextCallSeq() { @@ -1332,8 +1338,7 @@ public int getScannersCount() { return scanners.size(); } - public - RegionScanner getScanner(long scannerId) { + public RegionScanner getScanner(long scannerId) { String scannerIdString = Long.toString(scannerId); RegionScannerHolder scannerHolder = scanners.get(scannerIdString); if (scannerHolder != null) { @@ -1342,6 +1347,20 @@ RegionScanner getScanner(long scannerId) { return null; } + public long getScannerExpirationDelayMs(Long scannerId) { + if (scannerId == null) { + return this.scannerLeaseTimeoutPeriod; // This is a new scanner. + } + RegionScannerHolder scannerHolder = scanners.get(Long.toString(scannerId)); + if (scannerHolder != null) { + return scannerHolder.getExpirationDelayMs(); + } + // This is a missing/expired scanner. + // Return a large value; ideally it should be Long.MAX_VALUE but we don't want to rely on + // the correct overflow handling by the caller when the code there changes. + return TimeUnit.DAYS.toMillis(1); + } + public String getScanDetailsWithId(long scannerId) { RegionScanner scanner = getScanner(scannerId); if (scanner == null) { @@ -1418,8 +1437,8 @@ private RegionScannerHolder addScanner(String scannerName, RegionScanner s, Ship } else { closeCallback = new RegionScannerCloseCallBack(s); } - RegionScannerHolder rsh = - new RegionScannerHolder(scannerName, s, r, closeCallback, shippedCallback, needCursor); + RegionScannerHolder rsh = new RegionScannerHolder( + scannerName, s, r, closeCallback, shippedCallback, needCursor, lease); RegionScannerHolder existing = scanners.putIfAbsent(scannerName, rsh); assert existing == null : "scannerId must be unique within regionserver's whole lifecycle! " + scannerName; @@ -3795,4 +3814,8 @@ protected AccessChecker getAccessChecker() { protected ZKPermissionWatcher getZkPermissionWatcher() { return zkPermissionWatcher; } + + public boolean isMaster() { + return false; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java index 2f4dce88fb61..5bfaafb7eaf3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java @@ -29,7 +29,10 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; +import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.ipc.PriorityFunction; +import org.apache.hadoop.hbase.master.LoadBalancer; +import org.apache.hadoop.hbase.regionserver.AnnotationReadingPriorityFunction.ScanDeadlineOnly; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.RegionServerTests; @@ -116,9 +119,7 @@ public void testQosFunctionWithoutKnownArgument() throws IOException { //known argument classes (it uses one random request class) //(known argument classes are listed in //HRegionServer.QosFunctionImpl.knownArgumentClasses) - RequestHeader.Builder headerBuilder = RequestHeader.newBuilder(); - headerBuilder.setMethodName("foo"); - RequestHeader header = headerBuilder.build(); + RequestHeader header = createRequestHeader("foo"); PriorityFunction qosFunc = regionServer.rpcServices.getPriority(); assertEquals(HConstants.NORMAL_QOS, qosFunc.getPriority(header, null, User.createUserForTesting(regionServer.conf, "someuser", new String[]{"somegroup"}))); @@ -126,9 +127,7 @@ public void testQosFunctionWithoutKnownArgument() throws IOException { @Test public void testQosFunctionForScanMethod() throws IOException { - RequestHeader.Builder headerBuilder = RequestHeader.newBuilder(); - headerBuilder.setMethodName("Scan"); - RequestHeader header = headerBuilder.build(); + RequestHeader header = createRequestHeader("Scan"); //build an empty scan request ScanRequest.Builder scanBuilder = ScanRequest.newBuilder(); @@ -172,4 +171,124 @@ public void testQosFunctionForScanMethod() throws IOException { assertEquals(HConstants.NORMAL_QOS, priority.getPriority(header, scanRequest, User.createUserForTesting(regionServer.conf, "someuser", new String[]{"somegroup"}))); } + + private static HRegionServer prepareDeadlineTest( + TableName tableName, boolean isMeta, long newDelay) throws Exception { + HRegionServer mockRS = Mockito.mock(HRegionServer.class); + Configuration conf = HBaseConfiguration.create(); + conf.setBoolean("hbase.testing.nocluster", true); + HRegion mockRegion = Mockito.mock(HRegion.class); + RSRpcServices mockRpc = Mockito.mock(RSRpcServices.class); + Mockito.when(mockRS.getRSRpcServices()).thenReturn(mockRpc); + RegionInfo mockRegionInfo = Mockito.mock(RegionInfo.class); + Mockito.when(mockRpc.getRegion(Mockito.any())).thenReturn(mockRegion); + Mockito.when(mockRpc.isMaster()).thenReturn(false); + Mockito.when(mockRpc.getConfiguration()).thenReturn(conf); + Mockito.when(mockRegion.getRegionInfo()).thenReturn(mockRegionInfo); + Mockito.when(mockRegionInfo.getTable()).thenReturn(tableName); + Mockito.when(mockRegionInfo.isMetaRegion()).thenReturn(isMeta); + TableDescriptor td = Mockito.mock(TableDescriptor.class); + Mockito.when(td.isMetaRegion()).thenReturn(isMeta); + Mockito.when(mockRegion.getTableDescriptor()).thenReturn(td); + Mockito.when(mockRpc.getScannerExpirationDelayMs(null)).thenReturn(newDelay); + return mockRS; + } + + private void addMockScanner( + RSRpcServices mockRpc, long id, long delayMs, long vtime) throws Exception { + RegionScanner mockRegionScanner = Mockito.mock(RegionScanner.class); + Mockito.when(mockRpc.getScanner(id)).thenReturn(mockRegionScanner); + HRegion mockRegion = mockRpc.getRegion(null); + RegionInfo mockRi = mockRegion.getRegionInfo(); + Mockito.when(mockRegionScanner.getRegionInfo()).thenReturn(mockRi); + Mockito.when(mockRpc.getScannerExpirationDelayMs(id)).thenReturn(delayMs); + Mockito.when(mockRpc.getScannerVirtualTime(id)).thenReturn(vtime); + } + + private AnnotationReadingPriorityFunction prepareDeadlineTestFn( + AnnotationReadingPriorityFunction.ScanDeadlineOnly cfg, RSRpcServices rpc) { + rpc.getConfiguration().set( + AnnotationReadingPriorityFunction.SCAN_DEADLINE_PRIORITY, cfg.name()); + return new AnnotationReadingPriorityFunction(rpc); + } + + private static RequestHeader createRequestHeader(String type) { + RequestHeader.Builder headerBuilder = RequestHeader.newBuilder(); + headerBuilder.setMethodName(type); + return headerBuilder.build(); + } + + @Test + public void testScanDeadline() throws Exception { + RequestHeader header = createRequestHeader("Scan"); + + ScanRequest.Builder scanBuilder = ScanRequest.newBuilder(); + RegionSpecifier.Builder regionSpecifierBuilder = RegionSpecifier.newBuilder(); + // Hack-ish - ERN will be passed directly to the mock and ignored, so we can supply no value. + regionSpecifierBuilder.setType(RegionSpecifierType.ENCODED_REGION_NAME); + regionSpecifierBuilder.setValue(ByteString.EMPTY); + scanBuilder.setRegion(regionSpecifierBuilder.build()); + ScanRequest newReqNoMeta = scanBuilder.build(); + + scanBuilder = ScanRequest.newBuilder(); + regionSpecifierBuilder = RegionSpecifier.newBuilder(); + regionSpecifierBuilder.setType(RegionSpecifierType.REGION_NAME); + regionSpecifierBuilder.setValue(UnsafeByteOperations.unsafeWrap( + RegionInfoBuilder.FIRST_META_REGIONINFO.getRegionName())); + scanBuilder.setRegion(regionSpecifierBuilder.build()); + ScanRequest newReqMeta = scanBuilder.build(); + + final long SCANNER_ID = 1; + scanBuilder = ScanRequest.newBuilder(); + scanBuilder.setScannerId(SCANNER_ID); + ScanRequest oldReq = scanBuilder.build(); // Meta-ness is derived from RSRpcServices + + final long VTIMESQRT = 10, ACTUAL_DELAY = 123, DEFAULT_DELAY = 456; + + HRegionServer mockRS = prepareDeadlineTest(TableName.valueOf("a"), false, DEFAULT_DELAY); + addMockScanner(mockRS.getRSRpcServices(), SCANNER_ID, ACTUAL_DELAY, VTIMESQRT * VTIMESQRT); + + // Test new reqs and non-meta scan here, as well as old non-meta scanner. + AnnotationReadingPriorityFunction fn = prepareDeadlineTestFn( + ScanDeadlineOnly.NEVER, mockRS.getRSRpcServices()); + assertEquals(0L, fn.getDeadline(header, newReqNoMeta)); + assertEquals(0L, fn.getDeadline(header, newReqMeta)); + assertEquals(VTIMESQRT, fn.getDeadline(header, oldReq)); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqNoMeta)); + assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqMeta)); + assertEquals(DEFAULT_DELAY + VTIMESQRT, fn.getDeadline(header, oldReq)); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALWAYS, mockRS.getRSRpcServices()); + assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqNoMeta)); + assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqMeta)); + assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); + + // Old scanner against meta. + mockRS = prepareDeadlineTest(RegionInfoBuilder.FIRST_META_REGIONINFO.getTable(), + true, DEFAULT_DELAY); + addMockScanner(mockRS.getRSRpcServices(), SCANNER_ID, ACTUAL_DELAY, VTIMESQRT * VTIMESQRT); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.NEVER, mockRS.getRSRpcServices()); + assertEquals(VTIMESQRT, fn.getDeadline(header, oldReq)); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALWAYS, mockRS.getRSRpcServices()); + assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); + + // Meta on master shortcut - just check the config. + mockRS = prepareDeadlineTest(RegionInfoBuilder.FIRST_META_REGIONINFO.getTable(), + true, DEFAULT_DELAY); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + Configuration conf = mockRS.getRSRpcServices().getConfiguration(); + conf.setBoolean(LoadBalancer.TABLES_ON_MASTER, true); + conf.setBoolean(LoadBalancer.SYSTEM_TABLES_ON_MASTER, true); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + Mockito.when(mockRS.getRSRpcServices().isMaster()).thenReturn(true); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.ALWAYS, fn.getScanDeadlineOnly()); + conf.setBoolean(LoadBalancer.TABLES_ON_MASTER, false); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + } } From 0780b8421ea66219f757ce0dec59846967a2b7df Mon Sep 17 00:00:00 2001 From: Sergey Shelukhin Date: Mon, 6 May 2019 15:22:30 -0700 Subject: [PATCH 2/2] HBASE-22346 scanner priorities/deadline units are invalid for non-huge scanners - CR feedback --- .../AnnotationReadingPriorityFunction.java | 34 ++++++++++--------- .../hbase/regionserver/TestPriorityRpc.java | 28 +++++++-------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java index 65a9d512f1ef..aba5e8a7fac9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AnnotationReadingPriorityFunction.java @@ -80,11 +80,11 @@ public class AnnotationReadingPriorityFunction implements PriorityFunction { /** When to use the actual time-based deadline for scanners */ public static final String SCAN_DEADLINE_PRIORITY = "hbase.ipc.server.scan.deadline.only"; - private static final ScanDeadlineOnly SCAN_DEADLINE_PRIORITY_DEFAULT = ScanDeadlineOnly.NEVER; + private static final ScanDeadlineOnly SCAN_DEADLINE_PRIORITY_DEFAULT = ScanDeadlineOnly.NONE; enum ScanDeadlineOnly { - ALWAYS, - META, - NEVER + ALL, + META_ONLY, + NONE } private static final ByteString META_PREFIX = ByteString.copyFrom( @@ -178,11 +178,11 @@ private static ScanDeadlineOnly getScanDeadlineOnlyConf( } catch (IllegalArgumentException ex) { LOG.warn("Invalid value for {} ({}); using the default", SCAN_DEADLINE_PRIORITY, val); } - if (result == ScanDeadlineOnly.META && LoadBalancer.isTablesOnMaster(conf) + if (result == ScanDeadlineOnly.META_ONLY && LoadBalancer.isTablesOnMaster(conf) && LoadBalancer.isSystemTablesOnlyOnMaster(conf) && rpcServices.isMaster()) { - result = ScanDeadlineOnly.ALWAYS; + result = ScanDeadlineOnly.ALL; } - if (result != ScanDeadlineOnly.NEVER) { + if (result != ScanDeadlineOnly.NONE) { LOG.info("Using deadline-based scanner priority {}", result); } return result; @@ -301,15 +301,17 @@ public long getDeadline(RequestHeader header, Message param) { if (param instanceof ScanRequest) { ScanRequest request = (ScanRequest)param; boolean useDeadline; - long addToVTime = 0; + long baseTime = 0; switch (scanDeadlineOnly) { - case ALWAYS: useDeadline = true; break; - case NEVER: useDeadline = false; break; - case META: { + case ALL: useDeadline = true; break; + case NONE: useDeadline = false; break; + case META_ONLY: { useDeadline = isMetaScan(request); - // Make sure non-meta scans are generally after meta scans; add the default scanner delay. + // If meta regions use real time and non-meta use vtime, make sure they are comparable + // and that non-meta scans are generally after meta scans; add the default scanner + // delay to map the former to real time. if (!useDeadline) { - addToVTime = rpcServices.getScannerExpirationDelayMs(null); + baseTime = rpcServices.getScannerExpirationDelayMs(null); } break; } @@ -319,13 +321,13 @@ public long getDeadline(RequestHeader header, Message param) { return rpcServices.getScannerExpirationDelayMs( request.hasScannerId() ? request.getScannerId() : null); } else if (!request.hasScannerId()) { - return addToVTime; + return baseTime; } else { // get the 'virtual time' of the scanner, and applies sqrt() to get a - // nice curve for the delay. More a scanner is used the less priority it gets. + // nice curve for the delay. The more a scanner is used the less priority it gets. // The weight is used to have more control on the delay. long vtime = rpcServices.getScannerVirtualTime(request.getScannerId()); - return addToVTime + Math.round(Math.sqrt(vtime * scanVirtualTimeWeight)); + return baseTime + Math.round(Math.sqrt(vtime * scanVirtualTimeWeight)); } } return 0; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java index 5bfaafb7eaf3..5420190ab8cf 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPriorityRpc.java @@ -250,15 +250,15 @@ public void testScanDeadline() throws Exception { // Test new reqs and non-meta scan here, as well as old non-meta scanner. AnnotationReadingPriorityFunction fn = prepareDeadlineTestFn( - ScanDeadlineOnly.NEVER, mockRS.getRSRpcServices()); + ScanDeadlineOnly.NONE, mockRS.getRSRpcServices()); assertEquals(0L, fn.getDeadline(header, newReqNoMeta)); assertEquals(0L, fn.getDeadline(header, newReqMeta)); assertEquals(VTIMESQRT, fn.getDeadline(header, oldReq)); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqNoMeta)); assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqMeta)); assertEquals(DEFAULT_DELAY + VTIMESQRT, fn.getDeadline(header, oldReq)); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALWAYS, mockRS.getRSRpcServices()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALL, mockRS.getRSRpcServices()); assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqNoMeta)); assertEquals(DEFAULT_DELAY, fn.getDeadline(header, newReqMeta)); assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); @@ -267,28 +267,28 @@ public void testScanDeadline() throws Exception { mockRS = prepareDeadlineTest(RegionInfoBuilder.FIRST_META_REGIONINFO.getTable(), true, DEFAULT_DELAY); addMockScanner(mockRS.getRSRpcServices(), SCANNER_ID, ACTUAL_DELAY, VTIMESQRT * VTIMESQRT); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.NEVER, mockRS.getRSRpcServices()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.NONE, mockRS.getRSRpcServices()); assertEquals(VTIMESQRT, fn.getDeadline(header, oldReq)); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALWAYS, mockRS.getRSRpcServices()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.ALL, mockRS.getRSRpcServices()); assertEquals(ACTUAL_DELAY, fn.getDeadline(header, oldReq)); // Meta on master shortcut - just check the config. mockRS = prepareDeadlineTest(RegionInfoBuilder.FIRST_META_REGIONINFO.getTable(), true, DEFAULT_DELAY); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); - assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META_ONLY, fn.getScanDeadlineOnly()); Configuration conf = mockRS.getRSRpcServices().getConfiguration(); conf.setBoolean(LoadBalancer.TABLES_ON_MASTER, true); conf.setBoolean(LoadBalancer.SYSTEM_TABLES_ON_MASTER, true); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); - assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META_ONLY, fn.getScanDeadlineOnly()); Mockito.when(mockRS.getRSRpcServices().isMaster()).thenReturn(true); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); - assertEquals(ScanDeadlineOnly.ALWAYS, fn.getScanDeadlineOnly()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.ALL, fn.getScanDeadlineOnly()); conf.setBoolean(LoadBalancer.TABLES_ON_MASTER, false); - fn = prepareDeadlineTestFn(ScanDeadlineOnly.META, mockRS.getRSRpcServices()); - assertEquals(ScanDeadlineOnly.META, fn.getScanDeadlineOnly()); + fn = prepareDeadlineTestFn(ScanDeadlineOnly.META_ONLY, mockRS.getRSRpcServices()); + assertEquals(ScanDeadlineOnly.META_ONLY, fn.getScanDeadlineOnly()); } }