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 <elserj@apache.org>
This commit is contained in:
Sakthi 2019-01-11 13:05:38 -08:00 committed by Josh Elser
parent 54c724f580
commit 93d4b95b3f
3 changed files with 71 additions and 28 deletions

View File

@ -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.");
}
}
}

View File

@ -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();
}
}

View File

@ -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());
}