From 1ec003ddc85c216fc9a309141bd29a30365db716 Mon Sep 17 00:00:00 2001 From: Sakthi Date: Thu, 26 Jul 2018 16:52:16 -0700 Subject: [PATCH] HBASE-20885 Removed entry for RPC quota from hbase:quota when RPC quota is removed --- .../hadoop/hbase/quotas/ThrottleSettings.java | 9 ++ .../hbase/quotas/GlobalQuotaSettingsImpl.java | 84 +++++------- .../apache/hadoop/hbase/quotas/TestQuotaAdmin.java | 150 ++++++++++++++++++++- 3 files changed, 188 insertions(+), 55 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/ThrottleSettings.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/ThrottleSettings.java index 8cee7cdb01c17047f240b7485112a04214b1f448..c415320cb5763f8f95d05d029d6928bcfdcdd0f5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/ThrottleSettings.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/ThrottleSettings.java @@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.TableName; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetQuotaRequest; @@ -48,6 +49,14 @@ class ThrottleSettings extends QuotaSettings { return proto.hasTimedQuota() ? proto.getTimedQuota().getSoftLimit() : -1; } + /** + * Returns a copy of the internal state of this + */ + @VisibleForTesting + QuotaProtos.ThrottleRequest getProto() { + return proto.toBuilder().build(); + } + public TimeUnit getTimeUnit() { return proto.hasTimedQuota() ? ProtobufUtil.toTimeUnit(proto.getTimedQuota().getTimeUnit()) : null; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/GlobalQuotaSettingsImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/GlobalQuotaSettingsImpl.java index 3893d00b83189d5561dbb8ce8791ce912fbff94d..abdf4c739c53ed91452534e81e03021166247ecb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/GlobalQuotaSettingsImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/GlobalQuotaSettingsImpl.java @@ -115,70 +115,43 @@ public class GlobalQuotaSettingsImpl extends GlobalQuotaSettings { validateQuotaTarget(other); // Propagate the Throttle - QuotaProtos.Throttle.Builder throttleBuilder = (throttleProto == null - ? null : throttleProto.toBuilder()); + // To prevent the "empty" row in QuotaTableUtil.QUOTA_TABLE_NAME + QuotaProtos.Throttle.Builder throttleBuilder = null; + if (other instanceof ThrottleSettings) { - if (throttleBuilder == null) { - throttleBuilder = QuotaProtos.Throttle.newBuilder(); - } ThrottleSettings otherThrottle = (ThrottleSettings) other; if (otherThrottle.proto.hasType()) { QuotaProtos.ThrottleRequest otherProto = otherThrottle.proto; + if (otherProto.hasTimedQuota()) { - if (otherProto.hasTimedQuota()) { - validateTimedQuota(otherProto.getTimedQuota()); - } + validateTimedQuota(otherProto.getTimedQuota()); + + throttleBuilder = (throttleProto == null ? + QuotaProtos.Throttle.newBuilder() : + throttleProto.toBuilder()); switch (otherProto.getType()) { - case REQUEST_NUMBER: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setReqNum(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearReqNum(); - } - break; - case REQUEST_SIZE: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setReqSize(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearReqSize(); - } - break; - case WRITE_NUMBER: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setWriteNum(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearWriteNum(); - } - break; - case WRITE_SIZE: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setWriteSize(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearWriteSize(); - } - break; - case READ_NUMBER: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setReadNum(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearReqNum(); - } - break; - case READ_SIZE: - if (otherProto.hasTimedQuota()) { - throttleBuilder.setReadSize(otherProto.getTimedQuota()); - } else { - throttleBuilder.clearReadSize(); - } - break; + case REQUEST_NUMBER: + throttleBuilder.setReqNum(otherProto.getTimedQuota()); + break; + case REQUEST_SIZE: + throttleBuilder.setReqSize(otherProto.getTimedQuota()); + break; + case WRITE_NUMBER: + throttleBuilder.setWriteNum(otherProto.getTimedQuota()); + break; + case WRITE_SIZE: + throttleBuilder.setWriteSize(otherProto.getTimedQuota()); + break; + case READ_NUMBER: + throttleBuilder.setReadNum(otherProto.getTimedQuota()); + break; + case READ_SIZE: + throttleBuilder.setReadSize(otherProto.getTimedQuota()); + break; } - } else { - clearThrottleBuilder(throttleBuilder); } - } else { - clearThrottleBuilder(throttleBuilder); } } @@ -216,6 +189,9 @@ public class GlobalQuotaSettingsImpl extends GlobalQuotaSettings { Boolean bypassGlobals = this.bypassGlobals; if (other instanceof QuotaGlobalsSettingsBypass) { + throttleBuilder = + (throttleProto == null ? QuotaProtos.Throttle.newBuilder() : throttleProto.toBuilder()); + bypassGlobals = ((QuotaGlobalsSettingsBypass) other).getBypass(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java index 572fb102aa53729a96266dfaac64e1c6839414e5..12a4199b3a3a0bd3bb8ff1f872027c4f398d1923 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaAdmin.java @@ -368,7 +368,7 @@ public class TestQuotaAdmin { } @Test - public void testSetModifyRemoveQuota() throws Exception { + public void testSetModifyRemoveSpaceQuota() throws Exception { Admin admin = TEST_UTIL.getAdmin(); final TableName tn = TableName.valueOf("sq_table2"); final long originalSizeLimit = 1024L * 1024L * 1024L * 1024L * 5L; // 5TB @@ -453,6 +453,154 @@ public class TestQuotaAdmin { assertEquals(expected, countResults(filter)); } + @Test + public void testSetGetRemoveRPCQuota() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + final TableName tn = TableName.valueOf("sq_table1"); + QuotaSettings settings = + QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + admin.setQuota(settings); + + // Verify the Quota in the table + verifyRecordPresentInQuotaTable(ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + + // Verify we can retrieve it via the QuotaRetriever API + verifyFetchableViaAPI(admin, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + + // Now, remove the quota + QuotaSettings removeQuota = QuotaSettingsFactory.unthrottleTable(tn); + admin.setQuota(removeQuota); + + // Verify that the record doesn't exist in the table + verifyRecordNotPresentInQuotaTable(); + + // Verify that we can also not fetch it via the API + verifyNotFetchableViaAPI(admin); + } + + @Test + public void testSetModifyRemoveRPCQuota() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + final TableName tn = TableName.valueOf("sq_table1"); + QuotaSettings settings = + QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + admin.setQuota(settings); + + // Verify the Quota in the table + verifyRecordPresentInQuotaTable(ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + + // Verify we can retrieve it via the QuotaRetriever API + verifyFetchableViaAPI(admin, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + + // Setting a limit and time unit should be reflected + QuotaSettings newSettings = + QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 3L, TimeUnit.DAYS); + admin.setQuota(newSettings); + + // Verify the new Quota in the table + verifyRecordPresentInQuotaTable(ThrottleType.REQUEST_SIZE, 3L, TimeUnit.DAYS); + + // Verify we can retrieve the new quota via the QuotaRetriever API + verifyFetchableViaAPI(admin, ThrottleType.REQUEST_SIZE, 3L, TimeUnit.DAYS); + + // Now, remove the quota + QuotaSettings removeQuota = QuotaSettingsFactory.unthrottleTable(tn); + admin.setQuota(removeQuota); + + // Verify that the record doesn't exist in the table + verifyRecordNotPresentInQuotaTable(); + + // Verify that we can also not fetch it via the API + verifyNotFetchableViaAPI(admin); + + } + + private void verifyRecordPresentInQuotaTable(ThrottleType type, long limit, TimeUnit tu) + throws Exception { + // Verify the RPC Quotas in the table + try (Table quotaTable = TEST_UTIL.getConnection().getTable(QuotaTableUtil.QUOTA_TABLE_NAME); + ResultScanner scanner = quotaTable.getScanner(new Scan())) { + Result r = Iterables.getOnlyElement(scanner); + CellScanner cells = r.cellScanner(); + assertTrue("Expected to find a cell", cells.advance()); + assertRPCQuota(type, limit, tu, cells.current()); + } + } + + private void verifyRecordNotPresentInQuotaTable() throws Exception { + // Verify that the record doesn't exist in the QuotaTableUtil.QUOTA_TABLE_NAME + try (Table quotaTable = TEST_UTIL.getConnection().getTable(QuotaTableUtil.QUOTA_TABLE_NAME); + ResultScanner scanner = quotaTable.getScanner(new Scan())) { + assertNull("Did not expect to find a quota entry", scanner.next()); + } + } + + private void verifyFetchableViaAPI(Admin admin, ThrottleType type, long limit, TimeUnit tu) + throws Exception { + // Verify we can retrieve the new quota via the QuotaRetriever API + try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) { + assertRPCQuota(type, limit, tu, Iterables.getOnlyElement(quotaScanner)); + } + } + + private void verifyNotFetchableViaAPI(Admin admin) throws Exception { + // Verify that we can also not fetch it via the API + try (QuotaRetriever quotaScanner = QuotaRetriever.open(admin.getConfiguration())) { + assertNull("Did not expect to find a quota entry", quotaScanner.next()); + } + } + + private void assertRPCQuota(ThrottleType type, long limit, TimeUnit tu, Cell cell) + throws Exception { + Quotas q = QuotaTableUtil + .quotasFromData(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()); + assertTrue("Quota should have rpc quota defined", q.hasThrottle()); + + QuotaProtos.Throttle rpcQuota = q.getThrottle(); + QuotaProtos.TimedQuota t = null; + + switch (type) { + case REQUEST_SIZE: + assertTrue(rpcQuota.hasReqSize()); + t = rpcQuota.getReqSize(); + break; + case READ_NUMBER: + assertTrue(rpcQuota.hasReadNum()); + t = rpcQuota.getReadNum(); + break; + case READ_SIZE: + assertTrue(rpcQuota.hasReadSize()); + t = rpcQuota.getReadSize(); + break; + case REQUEST_NUMBER: + assertTrue(rpcQuota.hasReqNum()); + t = rpcQuota.getReqNum(); + break; + case WRITE_NUMBER: + assertTrue(rpcQuota.hasWriteNum()); + t = rpcQuota.getWriteNum(); + break; + case WRITE_SIZE: + assertTrue(rpcQuota.hasWriteSize()); + t = rpcQuota.getWriteSize(); + break; + } + + assertEquals(t.getSoftLimit(), limit); + assertEquals(t.getTimeUnit(), ProtobufUtil.toProtoTimeUnit(tu)); + } + + private void assertRPCQuota(ThrottleType type, long limit, TimeUnit tu, + QuotaSettings actualSettings) throws Exception { + assertTrue( + "The actual QuotaSettings was not an instance of " + ThrottleSettings.class + " but of " + + actualSettings.getClass(), actualSettings instanceof ThrottleSettings); + QuotaProtos.ThrottleRequest throttleRequest = ((ThrottleSettings) actualSettings).getProto(); + assertEquals(limit, throttleRequest.getTimedQuota().getSoftLimit()); + assertEquals(ProtobufUtil.toProtoTimeUnit(tu), throttleRequest.getTimedQuota().getTimeUnit()); + assertEquals(ProtobufUtil.toProtoThrottleType(type), throttleRequest.getType()); + } + private void assertSpaceQuota( long sizeLimit, SpaceViolationPolicy violationPolicy, Cell cell) throws Exception { Quotas q = QuotaTableUtil.quotasFromData( -- 2.15.1 (Apple Git-101)