HBASE-17977 Enable the MasterSpaceQuotaObserver by default

It should be the normal case that HBase automatically deletes
quotas for deleted tables. Switch the Observer to be on by
default and add an option to instead prevent it from being added.


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/f1a99903
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/f1a99903
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/f1a99903

Branch: refs/heads/master
Commit: f1a9990328a65c49cdd10af069f67ff5ba5147f9
Parents: b971b44
Author: Josh Elser <els...@apache.org>
Authored: Tue May 2 14:58:10 2017 -0400
Committer: Josh Elser <els...@apache.org>
Committed: Mon May 22 13:41:36 2017 -0400

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/master/HMaster.java | 28 +++++++
 .../hbase/quotas/MasterSpaceQuotaObserver.java  |  3 +
 .../quotas/TestMasterSpaceQuotaObserver.java    | 15 +++-
 .../TestMasterSpaceQuotaObserverWithMocks.java  | 84 ++++++++++++++++++++
 src/main/asciidoc/_chapters/ops_mgt.adoc        | 11 ++-
 5 files changed, 134 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/f1a99903/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index 89f085e..cd96552 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -134,6 +134,7 @@ import org.apache.hadoop.hbase.procedure2.ProcedureEvent;
 import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
 import org.apache.hadoop.hbase.procedure2.store.wal.WALProcedureStore;
 import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.MasterSpaceQuotaObserver;
 import org.apache.hadoop.hbase.quotas.QuotaObserverChore;
 import org.apache.hadoop.hbase.quotas.QuotaUtil;
 import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotNotifier;
@@ -794,6 +795,11 @@ public class HMaster extends HRegionServer implements 
MasterServices {
             conf, this.clusterConnection);
     tableCFsUpdater.update();
 
+    // Add the Observer to delete space quotas on table deletion before 
starting all CPs by
+    // default with quota support, avoiding if user specifically asks to not 
load this Observer.
+    if (QuotaUtil.isQuotaEnabled(conf)) {
+      updateConfigurationForSpaceQuotaObserver(conf);
+    }
     // initialize master side coprocessors before we start handling requests
     status.setStatus("Initializing master coprocessors");
     this.cpHost = new MasterCoprocessorHost(this, this.conf);
@@ -942,6 +948,28 @@ public class HMaster extends HRegionServer implements 
MasterServices {
     zombieDetector.interrupt();
   }
 
+  /**
+   * Adds the {@code MasterSpaceQuotaObserver} to the list of configured 
Master observers to
+   * automatically remove space quotas for a table when that table is deleted.
+   */
+  @VisibleForTesting
+  public void updateConfigurationForSpaceQuotaObserver(Configuration conf) {
+    // We're configured to not delete quotas on table deletion, so we don't 
need to add the obs.
+    if (!conf.getBoolean(
+          MasterSpaceQuotaObserver.REMOVE_QUOTA_ON_TABLE_DELETE,
+          MasterSpaceQuotaObserver.REMOVE_QUOTA_ON_TABLE_DELETE_DEFAULT)) {
+      return;
+    }
+    String[] masterCoprocs = 
conf.getStrings(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY);
+    final int length = null == masterCoprocs ? 0 : masterCoprocs.length;
+    String[] updatedCoprocs = new String[length + 1];
+    if (length > 0) {
+      System.arraycopy(masterCoprocs, 0, updatedCoprocs, 0, 
masterCoprocs.length);
+    }
+    updatedCoprocs[length] = MasterSpaceQuotaObserver.class.getName();
+    conf.setStrings(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, 
updatedCoprocs);
+  }
+
   private void initMobCleaner() {
     this.expiredMobFileCleanerChore = new ExpiredMobFileCleanerChore(this);
     getChoreService().scheduleChore(expiredMobFileCleanerChore);

http://git-wip-us.apache.org/repos/asf/hbase/blob/f1a99903/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java
index 299ba39..7c86525 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java
@@ -36,6 +36,9 @@ import 
org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.Quotas;
  */
 @InterfaceAudience.Private
 public class MasterSpaceQuotaObserver implements MasterObserver {
+  public static final String REMOVE_QUOTA_ON_TABLE_DELETE = 
"hbase.quota.remove.on.table.delete";
+  public static final boolean REMOVE_QUOTA_ON_TABLE_DELETE_DEFAULT = true;
+
   private CoprocessorEnvironment cpEnv;
   private Configuration conf;
   private boolean quotasEnabled = false;

http://git-wip-us.apache.org/repos/asf/hbase/blob/f1a99903/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java
index ea59d70..19c74ed 100644
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java
@@ -17,8 +17,10 @@
 package org.apache.hadoop.hbase.quotas;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.hadoop.conf.Configuration;
@@ -30,6 +32,8 @@ 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.CoprocessorHost;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.junit.AfterClass;
 import org.junit.Before;
@@ -53,7 +57,6 @@ public class TestMasterSpaceQuotaObserver {
   @BeforeClass
   public static void setUp() throws Exception {
     Configuration conf = TEST_UTIL.getConfiguration();
-    conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, 
MasterSpaceQuotaObserver.class.getName());
     conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
     TEST_UTIL.startMiniCluster(1);
   }
@@ -134,6 +137,16 @@ public class TestMasterSpaceQuotaObserver {
     assertEquals(0, getNumSpaceQuotas());
   }
 
+  @Test
+  public void testObserverAddedByDefault() throws Exception {
+    final HMaster master = TEST_UTIL.getHBaseCluster().getMaster();
+    final MasterCoprocessorHost cpHost = master.getMasterCoprocessorHost();
+    Set<String> coprocessorNames = cpHost.getCoprocessors();
+    assertTrue(
+        "Did not find MasterSpaceQuotaObserver in list of CPs: " + 
coprocessorNames,
+        
coprocessorNames.contains(MasterSpaceQuotaObserver.class.getSimpleName()));
+  }
+
   public boolean namespaceExists(String ns) throws IOException {
     NamespaceDescriptor[] descs = 
TEST_UTIL.getAdmin().listNamespaceDescriptors();
     for (NamespaceDescriptor desc : descs) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/f1a99903/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java
new file mode 100644
index 0000000..271e5bb
--- /dev/null
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import static 
org.apache.hadoop.hbase.coprocessor.CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY;
+import static 
org.apache.hadoop.hbase.quotas.MasterSpaceQuotaObserver.REMOVE_QUOTA_ON_TABLE_DELETE;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.mock;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.security.access.AccessController;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test class for MasterSpaceQuotaObserver that does not require a cluster.
+ */
+@Category(SmallTests.class)
+public class TestMasterSpaceQuotaObserverWithMocks {
+
+  private HMaster master;
+  private Configuration conf;
+
+  @Before
+  public void setup() {
+    conf = HBaseConfiguration.create();
+    master = mock(HMaster.class);
+    doCallRealMethod().when(master).updateConfigurationForSpaceQuotaObserver(
+        any(Configuration.class));
+  }
+
+  @Test
+  public void testAddDefaultObserver() {
+    master.updateConfigurationForSpaceQuotaObserver(conf);
+    assertEquals(MasterSpaceQuotaObserver.class.getName(), 
conf.get(MASTER_COPROCESSOR_CONF_KEY));
+  }
+
+  @Test
+  public void testDoNotAddDefaultObserver() {
+    conf.setBoolean(REMOVE_QUOTA_ON_TABLE_DELETE, false);
+    master.updateConfigurationForSpaceQuotaObserver(conf);
+    // Configuration#getStrings returns null when unset
+    assertNull(conf.getStrings(MASTER_COPROCESSOR_CONF_KEY));
+  }
+
+  @Test
+  public void testAppendsObserver() {
+    conf.set(MASTER_COPROCESSOR_CONF_KEY, AccessController.class.getName());
+    master.updateConfigurationForSpaceQuotaObserver(conf);
+    Set<String> coprocs = new 
HashSet<>(conf.getStringCollection(MASTER_COPROCESSOR_CONF_KEY));
+    assertEquals(2, coprocs.size());
+    assertTrue(
+        "Observed coprocessors were: " + coprocs,
+        coprocs.contains(AccessController.class.getName()));
+    assertTrue(
+        "Observed coprocessors were: " + coprocs,
+        coprocs.contains(MasterSpaceQuotaObserver.class.getName()));
+  }
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/f1a99903/src/main/asciidoc/_chapters/ops_mgt.adoc
----------------------------------------------------------------------
diff --git a/src/main/asciidoc/_chapters/ops_mgt.adoc 
b/src/main/asciidoc/_chapters/ops_mgt.adoc
index f60395b..b26e44b 100644
--- a/src/main/asciidoc/_chapters/ops_mgt.adoc
+++ b/src/main/asciidoc/_chapters/ops_mgt.adoc
@@ -1949,19 +1949,18 @@ on 'ns1:t1', this table can grow up to 100TB, but only 
if 'ns1:t2' and 'ns1:t3'
 Practically, it's limit is 100TB less the current usage of 'ns1:t2' and 
'ns1:t3'.
 
 [[ops.space.quota.deletion]]
-=== Automatic Space Quota Deletion
+=== Disabling Automatic Space Quota Deletion
 
 By default, if a table or namespace is deleted that has a space quota, the 
quota itself is
-not also deleted. In some cases, it may be desirable for the space quota to be 
automatically deleted.
-In these cases, the user may configure the MasterSpaceQuotaObserver to delete 
any space quota
-automatically in hbase-site.xml.
+also deleted. In some cases, it may be desirable for the space quota to not be 
automatically deleted.
+In these cases, the user may configure the system to not delete any space 
quota automatically via hbase-site.xml.
 
 [source,java]
 ----
 
   <property>
-    <name>hbase.coprocessor.master.classes</name>
-    <value>...,org.apache.hadoop.hbase.quotas.MasterSpaceQuotaObserver</value>
+    <name>hbase.master.quota.observer.ignore</name>
+    <value>true</value>
   </property>
 ----
 

Reply via email to