From 33102a1265f9200b99553a44a9d70d1e420b9303 Mon Sep 17 00:00:00 2001 From: surbhi Date: Fri, 19 Jun 2020 16:21:22 -0700 Subject: [PATCH] HBASE-22146 Removing a namespace-level space quota does not remove policies against contained tables Closes #1935 Signed-off-by: Josh Elser --- .../hadoop/hbase/quotas/QuotaTableUtil.java | 28 ++++++++ .../apache/hadoop/hbase/quotas/QuotaUtil.java | 8 +++ .../quotas/SpaceQuotaHelperForTests.java | 64 ++++++++++++++++++- .../hbase/quotas/TestSpaceQuotaRemoval.java | 58 +++++++++++++++++ 4 files changed, 157 insertions(+), 1 deletion(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java index 4f1491136b6..624b4751b3d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/QuotaTableUtil.java @@ -628,6 +628,34 @@ public class QuotaTableUtil { } } + /** + * Remove table usage snapshots (u:p columns) for the namespace passed + * @param connection connection to re-use + * @param namespace the namespace to fetch the list of table usage snapshots + */ + static void deleteTableUsageSnapshotsForNamespace(Connection connection, String namespace) + throws IOException { + Scan s = new Scan(); + //Get rows for all tables in namespace + s.setRowPrefixFilter(Bytes.add(QUOTA_TABLE_ROW_KEY_PREFIX, Bytes.toBytes(namespace + TableName.NAMESPACE_DELIM))); + //Scan for table usage column (u:p) in quota table + s.addColumn(QUOTA_FAMILY_USAGE,QUOTA_QUALIFIER_POLICY); + //Scan for table quota column (q:s) if table has a space quota defined + s.addColumn(QUOTA_FAMILY_INFO,QUOTA_QUALIFIER_SETTINGS); + try (Table quotaTable = connection.getTable(QUOTA_TABLE_NAME); + ResultScanner rs = quotaTable.getScanner(s)) { + for (Result r : rs) { + byte[] data = r.getValue(QUOTA_FAMILY_INFO, QUOTA_QUALIFIER_SETTINGS); + //if table does not have a table space quota defined, delete table usage column (u:p) + if (data == null) { + Delete delete = new Delete(r.getRow()); + delete.addColumns(QUOTA_FAMILY_USAGE,QUOTA_QUALIFIER_POLICY); + quotaTable.delete(delete); + } + } + } + } + /** * Fetches the computed size of all snapshots against tables in a namespace for space quotas. */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java index 9053405b13e..1fc81e527e0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java @@ -266,6 +266,14 @@ public class QuotaUtil extends QuotaTableUtil { if (qualifier != null) { delete.addColumns(QUOTA_FAMILY_INFO, qualifier); } + if (isNamespaceRowKey(rowKey)) { + String ns = getNamespaceFromRowKey(rowKey); + Quotas namespaceQuota = getNamespaceQuota(connection,ns); + if (namespaceQuota != null && namespaceQuota.hasSpace()) { + // When deleting namespace space quota, also delete table usage(u:p) snapshots + deleteTableUsageSnapshotsForNamespace(connection, ns); + } + } doDelete(connection, delete); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java index 93367b8df6f..b06621584c4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java @@ -161,6 +161,28 @@ public class SpaceQuotaHelperForTests { return tn; } + + TableName writeUntilViolationAndVerifyViolationInNamespace( + String ns, SpaceViolationPolicy policyToViolate, Mutation m) throws Exception { + final TableName tn = writeUntilViolationInNamespace(ns, policyToViolate); + verifyViolation(policyToViolate, tn, m); + return tn; + } + + TableName writeUntilViolationInNamespace(String ns, SpaceViolationPolicy policyToViolate) throws Exception { + TableName tn = createTableWithRegions(ns,10); + + setQuotaLimit(ns, policyToViolate, 4L); + + // Write more data than should be allowed and flush it to disk + writeData(tn, 5L * SpaceQuotaHelperForTests.ONE_MEGABYTE); + + // This should be sufficient time for the chores to run and see the change. + Thread.sleep(5000); + + return tn; + } + /** * Verifies that the given policy on the given table has been violated */ @@ -263,6 +285,19 @@ public class SpaceQuotaHelperForTests { assertTrue("Expected to succeed in writing data to a table not having quota ", sawSuccess); } + /** + * Verifies that table usage snapshot exists for the table + */ + void verifyTableUsageSnapshotForSpaceQuotaExist(TableName tn) throws Exception { + boolean sawUsageSnapshot = false; + try (Table quotaTable = testUtil.getConnection().getTable(QuotaTableUtil.QUOTA_TABLE_NAME)) { + Scan s = QuotaTableUtil.makeQuotaSnapshotScanForTable(tn); + ResultScanner rs = quotaTable.getScanner(s); + sawUsageSnapshot = (rs.next() != null); + } + assertTrue("Expected to succeed in getting table usage snapshots for space quota", sawUsageSnapshot); + } + /** * Sets the given quota (policy & limit) on the passed table. */ @@ -274,6 +309,17 @@ public class SpaceQuotaHelperForTests { LOG.debug("Quota limit set for table = {}, limit = {}", tn, sizeLimit); } + /** + * Sets the given quota (policy & limit) on the passed namespace. + */ + void setQuotaLimit(String ns, SpaceViolationPolicy policy, long sizeInMBs) + throws Exception { + final long sizeLimit = sizeInMBs * SpaceQuotaHelperForTests.ONE_MEGABYTE; + QuotaSettings settings = QuotaSettingsFactory.limitNamespaceSpace(ns, sizeLimit, policy); + testUtil.getAdmin().setQuota(settings); + LOG.debug("Quota limit set for namespace = {}, limit = {}", ns, sizeLimit); + } + /** * Removes the space quota from the given table */ @@ -363,6 +409,16 @@ public class SpaceQuotaHelperForTests { // }; // } + /** + * Removes the space quota from the given namespace + */ + void removeQuotaFromNamespace(String ns) throws Exception { + QuotaSettings removeQuota = QuotaSettingsFactory.removeNamespaceSpaceLimit(ns); + Admin admin = testUtil.getAdmin(); + admin.setQuota(removeQuota); + LOG.debug("Space quota settings removed from the namespace ", ns); + } + /** * Removes all quotas defined in the HBase quota table. */ @@ -494,7 +550,13 @@ public class SpaceQuotaHelperForTests { } NamespaceDescriptor createNamespace() throws Exception { - NamespaceDescriptor nd = NamespaceDescriptor.create("ns" + counter.getAndIncrement()).build(); + return createNamespace(null); + } + + NamespaceDescriptor createNamespace(String namespace) throws Exception { + if (namespace == null || namespace.trim().isEmpty()) + namespace = "ns" + counter.getAndIncrement(); + NamespaceDescriptor nd = NamespaceDescriptor.create(namespace).build(); testUtil.getAdmin().createNamespace(nd); return nd; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaRemoval.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaRemoval.java index ba89990a053..3ed6c716ec8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaRemoval.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaRemoval.java @@ -20,6 +20,7 @@ import java.util.concurrent.atomic.AtomicLong; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.testclassification.LargeTests; @@ -139,6 +140,63 @@ public class TestSpaceQuotaRemoval { helper.verifyNoViolation(tn, put); } + @Test + public void testDeleteTableUsageSnapshotsForNamespace() throws Exception { + Put put = new Put(Bytes.toBytes("to_reject")); + put.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"), + Bytes.toBytes("reject")); + + SpaceViolationPolicy policy = SpaceViolationPolicy.NO_INSERTS; + + //Create a namespace + String ns1 = "nsnew"; + NamespaceDescriptor nsd = helper.createNamespace(ns1); + + //Create 2nd namespace with name similar to ns1 + String ns2 = ns1 + "test"; + NamespaceDescriptor nsd2 = helper.createNamespace(ns2); + + // Do puts until we violate space policy on table tn1 in namesapce ns1 + final TableName tn1 = helper.writeUntilViolationAndVerifyViolationInNamespace(ns1, policy, put); + + // Do puts until we violate space policy on table tn2 in namespace ns2 + final TableName tn2 = helper.writeUntilViolationAndVerifyViolationInNamespace(ns2, policy, put); + + // Now, remove the quota from namespace ns1 which will remove table usage snapshots for ns1 + helper.removeQuotaFromNamespace(ns1); + + // Verify that table usage snapshot for table tn2 in namespace ns2 exist + helper.verifyTableUsageSnapshotForSpaceQuotaExist(tn2); + + // Put a new row on tn2: should violate as space quota exists on namespace ns2 + helper.verifyViolation(policy, tn2, put); + + // Put a new row on tn1: should not violate as quota settings removed from namespace ns1 + helper.verifyNoViolation(tn1, put); + } + + @Test + public void testSetNamespaceSizeQuotaAndThenRemove() throws Exception { + Put put = new Put(Bytes.toBytes("to_reject")); + put.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"), + Bytes.toBytes("reject")); + + SpaceViolationPolicy policy = SpaceViolationPolicy.NO_INSERTS; + + //Create namespace + NamespaceDescriptor nsd = helper.createNamespace(); + String ns = nsd.getName(); + + // Do puts until we violate space policy on table tn1 + final TableName tn1 = helper.writeUntilViolationAndVerifyViolationInNamespace(ns, policy, put); + + // Now, remove the quota from namespace + helper.removeQuotaFromNamespace(ns); + + // Put a new row now on tn1: should not violate as quota settings removed from namespace + helper.verifyNoViolation(tn1, put); + } + private void setQuotaAndThenRemoveInOneAmongTwoTables(SpaceViolationPolicy policy) throws Exception { Put put = new Put(Bytes.toBytes("to_reject"));