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.

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);

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;

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) {

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

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>
 ----