From 64a5289d57ba1d71da11b73d5ed707b157f3ea02 Mon Sep 17 00:00:00 2001 From: Sakthi Date: Thu, 2 Aug 2018 23:50:42 -0700 Subject: [PATCH] HBASE-20813 Removed RPC quotas when the associated table/Namespace is dropped off --- .../org/apache/hadoop/hbase/master/HMaster.java | 18 ++-- .../hadoop/hbase/quotas/MasterQuotasObserver.java | 107 +++++++++++++++++++++ ...Observer.java => TestMasterQuotasObserver.java} | 106 ++++++++++++++++---- ...java => TestMasterQuotasObserverWithMocks.java} | 20 ++-- 4 files changed, 212 insertions(+), 39 deletions(-) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java rename hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/{TestMasterSpaceQuotaObserver.java => TestMasterQuotasObserver.java} (65%) rename hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/{TestMasterSpaceQuotaObserverWithMocks.java => TestMasterQuotasObserverWithMocks.java} (78%) 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 b7148d5884220a8231bb13fa2022a62778e09580..abea1486762b25ff86ebbc912cdbd0f9e49ad5bf 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 @@ -158,7 +158,7 @@ import org.apache.hadoop.hbase.procedure2.RemoteProcedureDispatcher.RemoteProced import org.apache.hadoop.hbase.procedure2.RemoteProcedureException; 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.MasterQuotasObserver; import org.apache.hadoop.hbase.quotas.QuotaObserverChore; import org.apache.hadoop.hbase.quotas.QuotaUtil; import org.apache.hadoop.hbase.quotas.SnapshotQuotaObserverChore; @@ -910,10 +910,10 @@ public class HMaster extends HRegionServer implements MasterServices { new ReplicationPeerConfigUpgrader(zooKeeper, conf); tableCFsUpdater.copyTableCFs(); - // Add the Observer to delete space quotas on table deletion before starting all CPs by + // Add the Observer to delete 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); + updateConfigurationForQuotasObserver(conf); } // initialize master side coprocessors before we start handling requests status.setStatus("Initializing master coprocessors"); @@ -1069,15 +1069,15 @@ public class HMaster extends HRegionServer implements MasterServices { } /** - * Adds the {@code MasterSpaceQuotaObserver} to the list of configured Master observers to - * automatically remove space quotas for a table when that table is deleted. + * Adds the {@code MasterQuotasObserver} to the list of configured Master observers to + * automatically remove quotas for a table when that table is deleted. */ @VisibleForTesting - public void updateConfigurationForSpaceQuotaObserver(Configuration conf) { + public void updateConfigurationForQuotasObserver(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)) { + MasterQuotasObserver.REMOVE_QUOTA_ON_TABLE_DELETE, + MasterQuotasObserver.REMOVE_QUOTA_ON_TABLE_DELETE_DEFAULT)) { return; } String[] masterCoprocs = conf.getStrings(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); @@ -1086,7 +1086,7 @@ public class HMaster extends HRegionServer implements MasterServices { if (length > 0) { System.arraycopy(masterCoprocs, 0, updatedCoprocs, 0, masterCoprocs.length); } - updatedCoprocs[length] = MasterSpaceQuotaObserver.class.getName(); + updatedCoprocs[length] = MasterQuotasObserver.class.getName(); conf.setStrings(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, updatedCoprocs); } 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 new file mode 100644 index 0000000000000000000000000000000000000000..20cabed69e9c38e3ed40784e691b088cc0664d5f --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotasObserver.java @@ -0,0 +1,107 @@ +/* + * 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 java.io.IOException; +import java.util.Optional; +import org.apache.hadoop.conf.Configuration; +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.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.shaded.protobuf.generated.QuotaProtos.Quotas; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * An observer to automatically delete quotas when a table/namespace + * is deleted. + */ +@InterfaceAudience.Private +public class MasterQuotasObserver implements MasterCoprocessor, 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; + + @Override + public Optional getMasterObserver() { + return Optional.of(this); + } + + @Override + public void start(CoprocessorEnvironment ctx) throws IOException { + this.cpEnv = ctx; + this.conf = cpEnv.getConfiguration(); + this.quotasEnabled = QuotaUtil.isQuotaEnabled(conf); + } + + @Override + public void postDeleteTable( + ObserverContext ctx, TableName tableName) throws IOException { + // Do nothing if quotas aren't enabled + if (!quotasEnabled) { + 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); + } + } + if (quotas.hasThrottle()){ + QuotaSettings settings = QuotaSettingsFactory.unthrottleTable(tableName); + try (Admin admin = conn.getAdmin()) { + admin.setQuota(settings); + } + } + } + } + + @Override + public void postDeleteNamespace( + ObserverContext ctx, String namespace) throws IOException { + // Do nothing if quotas aren't enabled + if (!quotasEnabled) { + return; + } + final Connection conn = ctx.getEnvironment().getConnection(); + Quotas quotas = QuotaUtil.getNamespaceQuota(conn, namespace); + if (quotas != null) { + if (quotas.hasSpace()) { + QuotaSettings settings = QuotaSettingsFactory.removeNamespaceSpaceLimit(namespace); + try (Admin admin = conn.getAdmin()) { + admin.setQuota(settings); + } + } + if (quotas.hasThrottle()) { + QuotaSettings settings = QuotaSettingsFactory.unthrottleNamespace(namespace); + try (Admin admin = conn.getAdmin()) { + admin.setQuota(settings); + } + } + } + } +} 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/TestMasterQuotasObserver.java similarity index 65% rename from hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java rename to hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java index 391b23873b407be283789607d5564efe596cf4b2..356475202f6583f1aa0ffdd4ca3eb6ef542e81dc 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/TestMasterQuotasObserver.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -32,7 +33,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; 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; @@ -46,14 +46,14 @@ import org.junit.experimental.categories.Category; import org.junit.rules.TestName; /** - * Test class for {@link MasterSpaceQuotaObserver}. + * Test class for {@link MasterQuotasObserver}. */ @Category(MediumTests.class) -public class TestMasterSpaceQuotaObserver { +public class TestMasterQuotasObserver { @ClassRule public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestMasterSpaceQuotaObserver.class); + HBaseClassTestRule.forClass(TestMasterQuotasObserver.class); private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); private static SpaceQuotaHelperForTests helper; @@ -90,36 +90,54 @@ public class TestMasterSpaceQuotaObserver { } @Test - public void testTableQuotaRemoved() throws Exception { + public void testTableSpaceQuotaRemoved() throws Exception { final Connection conn = TEST_UTIL.getConnection(); final Admin admin = conn.getAdmin(); final TableName tn = TableName.valueOf(testName.getMethodName()); // Drop the table if it somehow exists if (admin.tableExists(tn)) { - admin.disableTable(tn); - admin.deleteTable(tn); + dropTable(admin, tn); } - - // Create a table - HTableDescriptor tableDesc = new HTableDescriptor(tn); - tableDesc.addFamily(new HColumnDescriptor("F1")); - admin.createTable(tableDesc); + createTable(admin, tn); assertEquals(0, getNumSpaceQuotas()); - // Set a quota + // Set space quota QuotaSettings settings = QuotaSettingsFactory.limitTableSpace( tn, 1024L, SpaceViolationPolicy.NO_INSERTS); admin.setQuota(settings); assertEquals(1, getNumSpaceQuotas()); - // Delete the table and observe the quota being automatically deleted as well - admin.disableTable(tn); - admin.deleteTable(tn); + // Drop the table and observe the Space quota being automatically deleted as well + dropTable(admin, tn); assertEquals(0, getNumSpaceQuotas()); } @Test - public void testNamespaceQuotaRemoved() throws Exception { + public void testTableRPCQuotaRemoved() throws Exception { + final Connection conn = TEST_UTIL.getConnection(); + final Admin admin = conn.getAdmin(); + final TableName tn = TableName.valueOf(testName.getMethodName()); + // Drop the table if it somehow exists + if (admin.tableExists(tn)) { + dropTable(admin, tn); + } + + createTable(admin, tn); + assertEquals(0, getThrottleQuotas()); + + // Set RPC quota + QuotaSettings settings = QuotaSettingsFactory.throttleTable(tn, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + admin.setQuota(settings); + + assertEquals(1, getThrottleQuotas()); + + // Delete the table and observe the RPC quota being automatically deleted as well + dropTable(admin, tn); + assertEquals(0, getThrottleQuotas()); + } + + @Test + public void testNamespaceSpaceQuotaRemoved() throws Exception { final Connection conn = TEST_UTIL.getConnection(); final Admin admin = conn.getAdmin(); final String ns = testName.getMethodName(); @@ -139,19 +157,44 @@ public class TestMasterSpaceQuotaObserver { admin.setQuota(settings); assertEquals(1, getNumSpaceQuotas()); - // Delete the table and observe the quota being automatically deleted as well + // Delete the namespace and observe the quota being automatically deleted as well admin.deleteNamespace(ns); assertEquals(0, getNumSpaceQuotas()); } + @Test + public void testNamespaceRPCQuotaRemoved() throws Exception { + final Connection conn = TEST_UTIL.getConnection(); + final Admin admin = conn.getAdmin(); + final String ns = testName.getMethodName(); + // Drop the ns if it somehow exists + if (namespaceExists(ns)) { + admin.deleteNamespace(ns); + } + + // Create the ns + NamespaceDescriptor desc = NamespaceDescriptor.create(ns).build(); + admin.createNamespace(desc); + assertEquals(0, getThrottleQuotas()); + + // Set a quota + QuotaSettings settings = QuotaSettingsFactory.throttleNamespace(ns, ThrottleType.REQUEST_SIZE, 2L, TimeUnit.HOURS); + admin.setQuota(settings); + assertEquals(1, getThrottleQuotas()); + + // Delete the namespace and observe the quota being automatically deleted as well + admin.deleteNamespace(ns); + assertEquals(0, getThrottleQuotas()); + } + @Test public void testObserverAddedByDefault() throws Exception { final HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); final MasterCoprocessorHost cpHost = master.getMasterCoprocessorHost(); Set coprocessorNames = cpHost.getCoprocessors(); assertTrue( - "Did not find MasterSpaceQuotaObserver in list of CPs: " + coprocessorNames, - coprocessorNames.contains(MasterSpaceQuotaObserver.class.getSimpleName())); + "Did not find MasterQuotasObserver in list of CPs: " + coprocessorNames, + coprocessorNames.contains(MasterQuotasObserver.class.getSimpleName())); } public boolean namespaceExists(String ns) throws IOException { @@ -174,4 +217,27 @@ public class TestMasterSpaceQuotaObserver { } return numSpaceQuotas; } + + public int getThrottleQuotas() throws Exception { + QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); + int throttleQuotas = 0; + for (QuotaSettings quotaSettings : scanner) { + if (quotaSettings.getQuotaType() == QuotaType.THROTTLE) { + throttleQuotas++; + } + } + return throttleQuotas; + } + + private void createTable(Admin admin, TableName tn) throws Exception { + // Create a table + HTableDescriptor tableDesc = new HTableDescriptor(tn); + tableDesc.addFamily(new HColumnDescriptor("F1")); + admin.createTable(tableDesc); + } + + private void dropTable(Admin admin, TableName tn) throws Exception { + admin.disableTable(tn); + admin.deleteTable(tn); + } } 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/TestMasterQuotasObserverWithMocks.java similarity index 78% rename from hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java rename to hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserverWithMocks.java index 9dd98545772f59e401b1f1ffc9c0d9e399e1be7d..42c61a38cde8089b82d4d02c19fcbfa7f5e4eed3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserverWithMocks.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserverWithMocks.java @@ -18,7 +18,7 @@ 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.apache.hadoop.hbase.quotas.MasterQuotasObserver.REMOVE_QUOTA_ON_TABLE_DELETE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -40,14 +40,14 @@ import org.junit.Test; import org.junit.experimental.categories.Category; /** - * Test class for MasterSpaceQuotaObserver that does not require a cluster. + * Test class for MasterQuotasObserver that does not require a cluster. */ @Category(SmallTests.class) -public class TestMasterSpaceQuotaObserverWithMocks { +public class TestMasterQuotasObserverWithMocks { @ClassRule public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestMasterSpaceQuotaObserverWithMocks.class); + HBaseClassTestRule.forClass(TestMasterQuotasObserverWithMocks.class); private HMaster master; private Configuration conf; @@ -56,20 +56,20 @@ public class TestMasterSpaceQuotaObserverWithMocks { public void setup() { conf = HBaseConfiguration.create(); master = mock(HMaster.class); - doCallRealMethod().when(master).updateConfigurationForSpaceQuotaObserver( + doCallRealMethod().when(master).updateConfigurationForQuotasObserver( any()); } @Test public void testAddDefaultObserver() { - master.updateConfigurationForSpaceQuotaObserver(conf); - assertEquals(MasterSpaceQuotaObserver.class.getName(), conf.get(MASTER_COPROCESSOR_CONF_KEY)); + master.updateConfigurationForQuotasObserver(conf); + assertEquals(MasterQuotasObserver.class.getName(), conf.get(MASTER_COPROCESSOR_CONF_KEY)); } @Test public void testDoNotAddDefaultObserver() { conf.setBoolean(REMOVE_QUOTA_ON_TABLE_DELETE, false); - master.updateConfigurationForSpaceQuotaObserver(conf); + master.updateConfigurationForQuotasObserver(conf); // Configuration#getStrings returns null when unset assertNull(conf.getStrings(MASTER_COPROCESSOR_CONF_KEY)); } @@ -77,7 +77,7 @@ public class TestMasterSpaceQuotaObserverWithMocks { @Test public void testAppendsObserver() { conf.set(MASTER_COPROCESSOR_CONF_KEY, AccessController.class.getName()); - master.updateConfigurationForSpaceQuotaObserver(conf); + master.updateConfigurationForQuotasObserver(conf); Set coprocs = new HashSet<>(conf.getStringCollection(MASTER_COPROCESSOR_CONF_KEY)); assertEquals(2, coprocs.size()); assertTrue( @@ -85,6 +85,6 @@ public class TestMasterSpaceQuotaObserverWithMocks { coprocs.contains(AccessController.class.getName())); assertTrue( "Observed coprocessors were: " + coprocs, - coprocs.contains(MasterSpaceQuotaObserver.class.getName())); + coprocs.contains(MasterQuotasObserver.class.getName())); } } -- 2.15.1 (Apple Git-101)