[hbase] 02/02: HBASE-22142 Drop table RegionSizes with namespace quota

[el...@apache.org]

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

commit f1d3b5442292a02faacf955feec00936d6731fc8
Author: shardul-cr7 <sh...@gmail.com>
AuthorDate: Fri Sep 20 10:57:15 2019 -0400

    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.
    
    Signed-off-by: Josh Elser <el...@apache.org>
---
 .../hadoop/hbase/quotas/MasterQuotaManager.java    |  9 ++++
 .../hadoop/hbase/quotas/MasterQuotasObserver.java  | 47 +++++++++++++------
 .../hbase/quotas/TestSpaceQuotaDropTable.java      | 52 ++++++++++++++++++++++
 3 files changed, 95 insertions(+), 13 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
index d17f1d5..c103c68 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
@@ -709,5 +709,14 @@ public class MasterQuotaManager implements RegionStateListener {
     }
     return numEntriesRemoved;
   }
+
+  /**
+   * 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));
+  }
 }
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java
index cad3129..1ddc9e3 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java
@@ -24,10 +24,14 @@ import org.apache.hadoop.hbase.CoprocessorEnvironment;
 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;
 
@@ -35,6 +39,7 @@ 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 class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
 
   @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 class MasterQuotasObserver implements MasterCoprocessor, MasterObserver {
       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);
+          }
         }
       }
     }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaDropTable.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaDropTable.java
index cb60d12..11b9ec6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotaDropTable.java
+++ b/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 class TestSpaceQuotaDropTable {
     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"),