This is an automated email from the ASF dual-hosted git repository.
elserj pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new dc50b57 HBASE-21225: Having RPC & Space quota on a table/Namespace
doesn't allow space quota to be removed using 'NONE'
dc50b57 is described below
commit dc50b570c63d36041f4afbb9e8d7e1769ac254d3
Author: Sakthi <sakthivel.azh...@gmail.com>
AuthorDate: Fri Jan 11 13:05:38 2019 -0800
HBASE-21225: Having RPC & Space quota on a table/Namespace doesn't allow
space quota to be removed using 'NONE'
Signed-off-by: Josh Elser <els...@apache.org>
---
.../hadoop/hbase/quotas/SpaceLimitSettings.java | 15 +++---
.../hbase/quotas/GlobalQuotaSettingsImpl.java | 23 +++-----
.../hbase/quotas/TestMasterQuotasObserver.java | 61 +++++++++++++++++++---
3 files changed, 71 insertions(+), 28 deletions(-)
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitSettings.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitSettings.java
index 8b31e94..69baf62 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitSettings.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitSettings.java
@@ -38,9 +38,7 @@ class SpaceLimitSettings extends QuotaSettings {
SpaceLimitSettings(TableName tableName, long sizeLimit, SpaceViolationPolicy
violationPolicy) {
super(null, Objects.requireNonNull(tableName), null);
- if (sizeLimit < 0L) {
- throw new IllegalArgumentException("Size limit must be a non-negative
value.");
- }
+ validateSizeLimit(sizeLimit);
proto = buildProtoAddQuota(sizeLimit,
Objects.requireNonNull(violationPolicy));
}
@@ -54,9 +52,7 @@ class SpaceLimitSettings extends QuotaSettings {
SpaceLimitSettings(String namespace, long sizeLimit, SpaceViolationPolicy
violationPolicy) {
super(null, null, Objects.requireNonNull(namespace));
- if (sizeLimit < 0L) {
- throw new IllegalArgumentException("Size limit must be a non-negative
value.");
- }
+ validateSizeLimit(sizeLimit);
proto = buildProtoAddQuota(sizeLimit,
Objects.requireNonNull(violationPolicy));
}
@@ -240,4 +236,11 @@ class SpaceLimitSettings extends QuotaSettings {
}
return this;
}
+
+ // Helper function to validate sizeLimit
+ private void validateSizeLimit(long sizeLimit) {
+ if (sizeLimit < 0L) {
+ throw new IllegalArgumentException("Size limit must be a non-negative
value.");
+ }
+ }
}
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 e47e4ff..78f52c4 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
@@ -121,6 +121,7 @@ public class GlobalQuotaSettingsImpl extends
GlobalQuotaSettings {
if (other instanceof ThrottleSettings) {
ThrottleSettings otherThrottle = (ThrottleSettings) other;
if (!otherThrottle.proto.hasType() ||
!otherThrottle.proto.hasTimedQuota()) {
+ // It means it's a remove request
// To prevent the "empty" row in QuotaTableUtil.QUOTA_TABLE_NAME
throttleBuilder = null;
} else {
@@ -186,6 +187,7 @@ public class GlobalQuotaSettingsImpl extends
GlobalQuotaSettings {
}
if (quotaToMerge.getRemove()) {
+ // It means it's a remove request
// Update the builder to propagate the removal
spaceBuilder.setRemove(true).clearSoftLimit().clearViolationPolicy();
} else {
@@ -195,21 +197,22 @@ public class GlobalQuotaSettingsImpl extends
GlobalQuotaSettings {
}
}
+ boolean removeSpaceBuilder =
+ (spaceBuilder == null) || (spaceBuilder.hasRemove() &&
spaceBuilder.getRemove());
+
Boolean bypassGlobals = this.bypassGlobals;
if (other instanceof QuotaGlobalsSettingsBypass) {
bypassGlobals = ((QuotaGlobalsSettingsBypass) other).getBypass();
}
- if (throttleBuilder == null &&
- (spaceBuilder == null || (spaceBuilder.hasRemove() &&
spaceBuilder.getRemove()))
- && bypassGlobals == null) {
+ if (throttleBuilder == null && removeSpaceBuilder && bypassGlobals ==
null) {
return null;
}
return new GlobalQuotaSettingsImpl(
getUserName(), getTableName(), getNamespace(),
(throttleBuilder == null ? null : throttleBuilder.build()),
bypassGlobals,
- (spaceBuilder == null ? null : spaceBuilder.build()));
+ (removeSpaceBuilder ? null : spaceBuilder.build()));
}
private void validateTimedQuota(final TimedQuota timedQuota) throws
IOException {
@@ -315,16 +318,4 @@ public class GlobalQuotaSettingsImpl extends
GlobalQuotaSettings {
}
return quotas;
}
-
- private void clearThrottleBuilder(QuotaProtos.Throttle.Builder builder) {
- builder.clearReadNum();
- builder.clearReadSize();
- builder.clearReqNum();
- builder.clearReqSize();
- builder.clearWriteNum();
- builder.clearWriteSize();
- builder.clearReadCapacityUnit();
- builder.clearReadCapacityUnit();
- builder.clearWriteCapacityUnit();
- }
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java
index 92e1d9d..155e2ed 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java
@@ -146,16 +146,13 @@ public class TestMasterQuotasObserver {
if (admin.tableExists(tn)) {
dropTable(admin, tn);
}
-
createTable(admin, tn);
assertEquals(0, getNumSpaceQuotas());
assertEquals(0, getThrottleQuotas());
-
// Set Both quotas
QuotaSettings settings =
QuotaSettingsFactory.limitTableSpace(tn, 1024L,
SpaceViolationPolicy.NO_INSERTS);
admin.setQuota(settings);
-
settings =
QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 2L,
TimeUnit.HOURS);
admin.setQuota(settings);
@@ -163,8 +160,34 @@ public class TestMasterQuotasObserver {
assertEquals(1, getNumSpaceQuotas());
assertEquals(1, getThrottleQuotas());
- // Delete the table and observe the quotas being automatically deleted as
well
+ // Remove Space quota
+ settings = QuotaSettingsFactory.removeTableSpaceLimit(tn);
+ admin.setQuota(settings);
+ assertEquals(0, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Set back the space quota
+ settings = QuotaSettingsFactory.limitTableSpace(tn, 1024L,
SpaceViolationPolicy.NO_INSERTS);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Remove the throttle quota
+ settings = QuotaSettingsFactory.unthrottleTable(tn);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(0, getThrottleQuotas());
+
+ // Set back the throttle quota
+ settings =
+ QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 2L,
TimeUnit.HOURS);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Drop the table and check that both the quotas have been dropped as well
dropTable(admin, tn);
+
assertEquals(0, getNumSpaceQuotas());
assertEquals(0, getThrottleQuotas());
}
@@ -225,7 +248,6 @@ public class TestMasterQuotasObserver {
public void testNamespaceSpaceAndRPCQuotaRemoved() throws Exception {
final Connection conn = TEST_UTIL.getConnection();
final Admin admin = conn.getAdmin();
- final TableName tn = TableName.valueOf(testName.getMethodName());
final String ns = testName.getMethodName();
// Drop the ns if it somehow exists
if (namespaceExists(ns)) {
@@ -235,6 +257,7 @@ public class TestMasterQuotasObserver {
// Create the ns
NamespaceDescriptor desc = NamespaceDescriptor.create(ns).build();
admin.createNamespace(desc);
+
assertEquals(0, getNumSpaceQuotas());
assertEquals(0, getThrottleQuotas());
@@ -250,8 +273,34 @@ public class TestMasterQuotasObserver {
assertEquals(1, getNumSpaceQuotas());
assertEquals(1, getThrottleQuotas());
- // Delete the namespace and observe the quotas being automatically deleted
as well
+ // Remove Space quota
+ settings = QuotaSettingsFactory.removeNamespaceSpaceLimit(ns);
+ admin.setQuota(settings);
+ assertEquals(0, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Set back the space quota
+ settings = QuotaSettingsFactory.limitNamespaceSpace(ns, 1024L,
SpaceViolationPolicy.NO_INSERTS);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Remove the throttle quota
+ settings = QuotaSettingsFactory.unthrottleNamespace(ns);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(0, getThrottleQuotas());
+
+ // Set back the throttle quota
+ settings =
+ QuotaSettingsFactory.throttleNamespace(ns, ThrottleType.REQUEST_SIZE,
2L, TimeUnit.HOURS);
+ admin.setQuota(settings);
+ assertEquals(1, getNumSpaceQuotas());
+ assertEquals(1, getThrottleQuotas());
+
+ // Delete the namespace and check that both the quotas have been dropped
as well
admin.deleteNamespace(ns);
+
assertEquals(0, getNumSpaceQuotas());
assertEquals(0, getThrottleQuotas());
}
