From b96952beef41226a051fec8add2ff2c803cf1abc Mon Sep 17 00:00:00 2001 From: Matteo Bertozzi Date: Mon, 27 Apr 2015 21:43:39 +0100 Subject: [PATCH] HBASE-13394 Failed to recreate a table when quota is enabled --- .../apache/hadoop/hbase/master/HMaster.java | 3 +- .../procedure/CreateTableProcedure.java | 5 ++ .../procedure/DeleteTableProcedure.java | 14 +--- .../master/procedure/ProcedureSyncWait.java | 12 ++++ .../hbase/namespace/TestNamespaceAuditor.java | 64 +++++++++++++++++++ 5 files changed, 83 insertions(+), 15 deletions(-) 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 9bd1dbb676b..98006ecd4b6 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 @@ -1344,8 +1344,7 @@ public class HMaster extends HRegionServer implements MasterServices, Server { HRegionInfo[] newRegions = ModifyRegionUtils.createHRegionInfos(hTableDescriptor, splitKeys); checkInitialized(); sanityCheckTableDescriptor(hTableDescriptor); - this.quotaManager.checkNamespaceTableAndRegionQuota(hTableDescriptor.getTableName(), - newRegions.length); + if (cpHost != null) { cpHost.preCreateTable(hTableDescriptor, newRegions); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java index dd6d38775d3..edaf7fb5351 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java @@ -284,6 +284,11 @@ public class CreateTableProcedure private void preCreate(final MasterProcedureEnv env) throws IOException, InterruptedException { + if (!getTableName().isSystemTable()) { + ProcedureSyncWait.getMasterQuotaManager(env) + .checkNamespaceTableAndRegionQuota(getTableName(), newRegions.size()); + } + final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); if (cpHost != null) { final HRegionInfo[] regions = newRegions == null ? null : diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java index 2582a1e0071..dfe70c720c5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java @@ -53,7 +53,6 @@ import org.apache.hadoop.hbase.protobuf.generated.MasterProcedureProtos; import org.apache.hadoop.hbase.protobuf.generated.MasterProcedureProtos.DeleteTableState; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; -import org.apache.hadoop.hbase.quotas.MasterQuotaManager; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.security.UserGroupInformation; @@ -406,17 +405,6 @@ public class DeleteTableProcedure protected static void deleteTableStates(final MasterProcedureEnv env, final TableName tableName) throws IOException { - getMasterQuotaManager(env).removeTableFromNamespaceQuota(tableName); - } - - private static MasterQuotaManager getMasterQuotaManager(final MasterProcedureEnv env) - throws IOException { - return ProcedureSyncWait.waitFor(env, "quota manager to be available", - new ProcedureSyncWait.Predicate() { - @Override - public MasterQuotaManager evaluate() throws IOException { - return env.getMasterServices().getMasterQuotaManager(); - } - }); + ProcedureSyncWait.getMasterQuotaManager(env).removeTableFromNamespaceQuota(tableName); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ProcedureSyncWait.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ProcedureSyncWait.java index 903dbd3591e..1eb00730f74 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ProcedureSyncWait.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ProcedureSyncWait.java @@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureResult; +import org.apache.hadoop.hbase.quotas.MasterQuotaManager; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.Threads; import org.apache.hadoop.hbase.zookeeper.MetaTableLocator; @@ -176,4 +177,15 @@ public final class ProcedureSyncWait { }); } } + + protected static MasterQuotaManager getMasterQuotaManager(final MasterProcedureEnv env) + throws IOException { + return ProcedureSyncWait.waitFor(env, "quota manager to be available", + new ProcedureSyncWait.Predicate() { + @Override + public MasterQuotaManager evaluate() throws IOException { + return env.getMasterServices().getMasterQuotaManager(); + } + }); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java index c86e0ffbbc6..cccdace7477 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.Collections; @@ -41,6 +42,7 @@ import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.MiniHBaseCluster; @@ -100,6 +102,7 @@ public class TestNamespaceAuditor { UTIL.getConfiguration().set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, MasterSyncObserver.class.getName()); Configuration conf = UTIL.getConfiguration(); + conf.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 5); conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true); conf.setClass("hbase.coprocessor.regionserver.classes", CPRegionServerObserver.class, RegionServerObserver.class); @@ -470,6 +473,58 @@ public class TestNamespaceAuditor { htable.close(); } + /* + * Create a table and make sure that the table creation fails after adding this table entry into + * namespace quota cache. Now correct the failure and recreate the table with same name. + * HBASE-13394 + */ + @Test(timeout = 180000) + public void testRecreateTableWithSameNameAfterFirstTimeFailure() throws Exception { + String nsp1 = prefix + "_testRecreateTable"; + NamespaceDescriptor nspDesc = + NamespaceDescriptor.create(nsp1) + .addConfiguration(TableNamespaceManager.KEY_MAX_REGIONS, "20") + .addConfiguration(TableNamespaceManager.KEY_MAX_TABLES, "1").build(); + ADMIN.createNamespace(nspDesc); + final TableName tableOne = TableName.valueOf(nsp1 + TableName.NAMESPACE_DELIM + "table1"); + byte[] columnFamily = Bytes.toBytes("info"); + HTableDescriptor tableDescOne = new HTableDescriptor(tableOne); + tableDescOne.addFamily(new HColumnDescriptor(columnFamily)); + MasterSyncObserver.throwExceptionInPreCreateTableHandler = true; + try { + try { + ADMIN.createTable(tableDescOne); + fail("Table " + tableOne.toString() + "creation should fail."); + } catch (Exception exp) { + LOG.error(exp); + } + assertFalse(ADMIN.tableExists(tableOne)); + + NamespaceTableAndRegionInfo nstate = getNamespaceState(nsp1); + assertEquals("First table creation failed in namespace so number of tables in namespace " + + "should be 0.", 0, nstate.getTables().size()); + + MasterSyncObserver.throwExceptionInPreCreateTableHandler = false; + try { + ADMIN.createTable(tableDescOne); + } catch (Exception e) { + fail("Table " + tableOne.toString() + "creation should succeed."); + LOG.error(e); + } + assertTrue(ADMIN.tableExists(tableOne)); + nstate = getNamespaceState(nsp1); + assertEquals("First table was created successfully so table size in namespace should " + + "be one now.", 1, nstate.getTables().size()); + } finally { + MasterSyncObserver.throwExceptionInPreCreateTableHandler = false; + if (ADMIN.tableExists(tableOne)) { + ADMIN.disableTable(tableOne); + deleteTable(tableOne); + } + ADMIN.deleteNamespace(nsp1); + } + } + private NamespaceTableAndRegionInfo getNamespaceState(String namespace) throws KeeperException, IOException { return getQuotaManager().getState(namespace); @@ -569,6 +624,7 @@ public class TestNamespaceAuditor { public static class MasterSyncObserver extends BaseMasterObserver { volatile CountDownLatch tableDeletionLatch; + static boolean throwExceptionInPreCreateTableHandler; @Override public void preDeleteTable(ObserverContext ctx, @@ -582,6 +638,14 @@ public class TestNamespaceAuditor { throws IOException { tableDeletionLatch.countDown(); } + + @Override + public void preCreateTableHandler(ObserverContext ctx, + HTableDescriptor desc, HRegionInfo[] regions) throws IOException { + if (throwExceptionInPreCreateTableHandler) { + throw new IOException("Throw exception as it is demanded."); + } + } } private void deleteTable(final TableName tableName) throws Exception {