From 90cf5b9ba2feb980c31837e6070c85a1b4cf9edc Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Mon, 27 Apr 2015 11:28:21 +0530 Subject: [PATCH] HBASE-13394 Failed to recreate a table when quota is enabled --- .../org/apache/hadoop/hbase/master/HMaster.java | 16 ++++-- .../master/procedure/CreateTableProcedure.java | 4 ++ .../master/procedure/DeleteTableProcedure.java | 13 +---- .../hbase/master/procedure/ProcedureSyncWait.java | 12 ++++ .../hbase/namespace/TestNamespaceAuditor.java | 64 ++++++++++++++++++++++ 5 files changed, 92 insertions(+), 17 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 7ecfd8d..63a6e49 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 @@ -1428,14 +1428,13 @@ public class HMaster extends HRegionServer implements MasterServices, Server { throw new MasterNotRunningException(); } - String namespace = hTableDescriptor.getTableName().getNamespaceAsString(); + TableName tableName = hTableDescriptor.getTableName(); + String namespace = tableName.getNamespaceAsString(); ensureNamespaceExists(namespace); HRegionInfo[] newRegions = ModifyRegionUtils.createHRegionInfos(hTableDescriptor, splitKeys); checkInitialized(); sanityCheckTableDescriptor(hTableDescriptor); - this.quotaManager.checkNamespaceTableAndRegionQuota(hTableDescriptor.getTableName(), - newRegions.length); if (cpHost != null) { cpHost.preCreateTable(hTableDescriptor, newRegions); } @@ -1451,8 +1450,15 @@ public class HMaster extends HRegionServer implements MasterServices, Server { hTableDescriptor, newRegions, latch)); latch.await(); } else { - this.service.submit(new CreateTableHandler(this, this.fileSystemManager, hTableDescriptor, - conf, newRegions, this).prepare()); + try { + this.quotaManager.checkNamespaceTableAndRegionQuota(tableName, newRegions.length); + this.service.submit(new CreateTableHandler(this, this.fileSystemManager, hTableDescriptor, + conf, newRegions, this).prepare()); + } catch (IOException e) { + this.quotaManager.removeTableFromNamespaceQuota(tableName); + LOG.error("Exception occurred while creating the table " + tableName.getNameAsString(), e); + throw e; + } } if (cpHost != null) { 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 2db04f3..ecea171 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 @@ -296,6 +296,10 @@ 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 b61c850..f3bbdda 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 @@ -406,17 +406,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 a57fa59..63d8a1e 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; @@ -177,4 +178,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 1f0a827..05a0294 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 @@ -15,6 +15,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; @@ -33,6 +34,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; @@ -93,6 +95,7 @@ public class TestNamespaceAuditor { public static void setupOnce() throws Exception, IOException { Configuration conf = UTIL.getConfiguration(); + conf.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 5); conf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, CustomObserver.class.getName()); conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, MasterSyncObserver.class.getName()); conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true); @@ -476,6 +479,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.throwExceptionInPreCreateTable = 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.throwExceptionInPreCreateTable = 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.throwExceptionInPreCreateTable = 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); @@ -575,6 +630,7 @@ public class TestNamespaceAuditor { public static class MasterSyncObserver extends BaseMasterObserver { volatile CountDownLatch tableDeletionLatch; + static boolean throwExceptionInPreCreateTable; @Override public void preDeleteTable(ObserverContext ctx, @@ -587,6 +643,14 @@ public class TestNamespaceAuditor { TableName tableName) throws IOException { tableDeletionLatch.countDown(); } + + @Override + public void preCreateTable(ObserverContext ctx, + HTableDescriptor desc, HRegionInfo[] regions) throws IOException { + if (throwExceptionInPreCreateTable) { + throw new IOException("Throw exception as it is demanded."); + } + } } private void deleteTable(final TableName tableName) throws Exception { -- 1.9.2.msysgit.0