HBASE-22142 Drop table RegionSizes with namespace quota

There was a bug in which we would not drop the RegionSizes
for a table in a namespace, where the namespace had a quota
on it. This allowed a scenario in which recreation of a table
inside of a namespace would unintentionally move into violation
despite the table being empty. Need to make sure the RegionSizes
are dropped on table deletion if there is _any_ quota applying
to that table.

Closes #598

Signed-off-by: Josh Elser <[email protected]>

Author: shardul-cr7

hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java

@@ -740,5 +740,14 @@ public void processFileArchivals(FileArchiveNotificationRequest request, Connect
       notifier.addArchivedFiles(filesWithSize);
     }
   }
+
+  /**
+   * Removes each region size entry where the RegionInfo references the provided TableName.
+   *
+   * @param tableName tableName.
+   */
+  public void removeRegionSizesForTable(TableName tableName) {
+    regionSizes.keySet().removeIf(regionInfo -> regionInfo.getTable().equals(tableName));
+  }
 }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java

@@ -24,17 +24,22 @@
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.coprocessor.CoprocessorException;
+import org.apache.hadoop.hbase.coprocessor.CoreCoprocessor;
+import org.apache.hadoop.hbase.coprocessor.HasMasterServices;
 import org.apache.hadoop.hbase.coprocessor.MasterCoprocessor;
 import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
 import org.apache.hadoop.hbase.coprocessor.MasterObserver;
 import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.Quotas;
 
 /**
  * An observer to automatically delete quotas when a table/namespace
  * is deleted.
  */
+@CoreCoprocessor
 @InterfaceAudience.Private
 public class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
   public static final String REMOVE_QUOTA_ON_TABLE_DELETE = "hbase.quota.remove.on.table.delete";
@@ -43,6 +48,7 @@ public class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
   private CoprocessorEnvironment cpEnv;
   private Configuration conf;
   private boolean quotasEnabled = false;
+  private MasterServices masterServices;
 
   @Override
   public Optional<MasterObserver> getMasterObserver() {
@@ -51,9 +57,19 @@ public Optional<MasterObserver> getMasterObserver() {
 
   @Override
   public void start(CoprocessorEnvironment ctx) throws IOException {
-    this.cpEnv = ctx;
-    this.conf = cpEnv.getConfiguration();
+    this.conf = ctx.getConfiguration();
     this.quotasEnabled = QuotaUtil.isQuotaEnabled(conf);
+
+    if (!(ctx instanceof MasterCoprocessorEnvironment)) {
+      throw new CoprocessorException("Must be loaded on master.");
+    }
+    // if running on master
+    MasterCoprocessorEnvironment mEnv = (MasterCoprocessorEnvironment) ctx;
+    if (mEnv instanceof HasMasterServices) {
+      this.masterServices = ((HasMasterServices) mEnv).getMasterServices();
+    } else {
+      throw new CoprocessorException("Must be loaded on a master having master services.");
+    }
   }
 
   @Override
@@ -64,18 +80,23 @@ public void postDeleteTable(
       return;
     }
     final Connection conn = ctx.getEnvironment().getConnection();
-    Quotas quotas = QuotaUtil.getTableQuota(conn, tableName);
-    if (quotas != null){
-      if (quotas.hasSpace()){
-        QuotaSettings settings = QuotaSettingsFactory.removeTableSpaceLimit(tableName);
-        try (Admin admin = conn.getAdmin()) {
-          admin.setQuota(settings);
+    Quotas tableQuotas = QuotaUtil.getTableQuota(conn, tableName);
+    Quotas namespaceQuotas = QuotaUtil.getNamespaceQuota(conn, tableName.getNamespaceAsString());
+    if (tableQuotas != null || namespaceQuotas != null) {
+      // Remove regions of table from space quota map.
+      this.masterServices.getMasterQuotaManager().removeRegionSizesForTable(tableName);
+      if (tableQuotas != null) {
+        if (tableQuotas.hasSpace()) {
+          QuotaSettings settings = QuotaSettingsFactory.removeTableSpaceLimit(tableName);
+          try (Admin admin = conn.getAdmin()) {
+            admin.setQuota(settings);
+          }
         }
-      }
-      if (quotas.hasThrottle()){
-        QuotaSettings settings = QuotaSettingsFactory.unthrottleTable(tableName);
-        try (Admin admin = conn.getAdmin()) {
-          admin.setQuota(settings);
+        if (tableQuotas.hasThrottle()) {
+          QuotaSettings settings = QuotaSettingsFactory.unthrottleTable(tableName);
+          try (Admin admin = conn.getAdmin()) {
+            admin.setQuota(settings);
+          }
         }
       }
     }

hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaDropTable.java

@@ -15,16 +15,23 @@
  */
 package org.apache.hadoop.hbase.quotas;
 
+import java.util.List;
+import java.util.Map;
 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.MetaTableAccessor;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.Waiter;
 import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -87,6 +94,51 @@ public void testSetQuotaAndThenDropTableWithDisable() throws Exception {
     setQuotaAndThenDropTable(SpaceViolationPolicy.DISABLE);
   }
 
+  @Test
+  public void testSetQuotaAndThenDropTableWithRegionReport() throws Exception {
+    final TableName tn = helper.createTable();
+    helper.setQuotaLimit(tn, SpaceViolationPolicy.NO_INSERTS, 1L);
+    helper.writeData(tn, 2L);
+
+    final HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
+    final MasterQuotaManager quotaManager = master.getMasterQuotaManager();
+
+    // Make sure the master has report for the table.
+    Waiter.waitFor(TEST_UTIL.getConfiguration(), 30 * 1000, new Waiter.Predicate<Exception>() {
+      @Override
+      public boolean evaluate() throws Exception {
+        Map<RegionInfo, Long> regionSizes = quotaManager.snapshotRegionSizes();
+        List<RegionInfo> tableRegions =
+            MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn);
+        return regionSizes.containsKey(tableRegions.get(0));
+      }
+    });
+
+    boolean hasRegionSize = false;
+
+    // region report should be present before dropping the table.
+    for (Map.Entry<RegionInfo, Long> entry : quotaManager.snapshotRegionSizes().entrySet()) {
+      if (entry.getKey().getTable().equals(tn)) {
+        hasRegionSize = true;
+        break;
+      }
+    }
+
+    // regionSize report for the given table should be present before dropping the table.
+    Assert.assertTrue(hasRegionSize);
+
+    // drop the table
+    TEST_UTIL.getAdmin().disableTable(tn);
+    TEST_UTIL.getAdmin().deleteTable(tn);
+
+    // check if deleted table region report still present in the map.
+    for (Map.Entry<RegionInfo, Long> entry : quotaManager.snapshotRegionSizes().entrySet()) {
+      if (entry.getKey().getTable().equals(tn)) {
+        Assert.fail("Dropped table regionSizes were not deleted during the drop command");
+      }
+    }
+  }
+
   private void setQuotaAndThenDropTable(SpaceViolationPolicy policy) throws Exception {
     Put put = new Put(Bytes.toBytes("to_reject"));
     put.addColumn(Bytes.toBytes(SpaceQuotaHelperForTests.F1), Bytes.toBytes("to"),