From 1a100ddab07590016683b4e61f3596537d5ae210 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Mon, 6 Apr 2015 12:00:36 +0530 Subject: [PATCH] HBASE-13394 Failed to recreate a table when quota is enabled --- .../org/apache/hadoop/hbase/master/HMaster.java | 24 +++++--- .../hbase/namespace/TestNamespaceAuditor.java | 66 +++++++++++++++++++++- 2 files changed, 80 insertions(+), 10 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 581e3c9..42344be 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 @@ -1287,21 +1287,27 @@ 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 = getHRegionInfos(hTableDescriptor, splitKeys); checkInitialized(); sanityCheckTableDescriptor(hTableDescriptor); - this.quotaManager.checkNamespaceTableAndRegionQuota(hTableDescriptor.getTableName(), - newRegions.length); - if (cpHost != null) { - cpHost.preCreateTable(hTableDescriptor, newRegions); + this.quotaManager.checkNamespaceTableAndRegionQuota(tableName, newRegions.length); + try { + if (cpHost != null) { + cpHost.preCreateTable(hTableDescriptor, newRegions); + } + LOG.info(getClientIdAuditPrefix() + " create " + hTableDescriptor); + + 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; } - LOG.info(getClientIdAuditPrefix() + " create " + hTableDescriptor); - this.service.submit(new CreateTableHandler(this, - this.fileSystemManager, hTableDescriptor, conf, - newRegions, this).prepare()); if (cpHost != null) { cpHost.postCreateTable(hTableDescriptor, newRegions); } 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 c86e0ff..e0b1563 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; @@ -103,6 +105,7 @@ public class TestNamespaceAuditor { conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true); conf.setClass("hbase.coprocessor.regionserver.classes", CPRegionServerObserver.class, RegionServerObserver.class); + conf.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 5); UTIL.startMiniCluster(1, 1); waitForQuotaEnabled(); ADMIN = UTIL.getHBaseAdmin(); @@ -540,6 +543,58 @@ public class TestNamespaceAuditor { assertEquals("Expected: " + before.getTables() + " Found: " + after.getTables(), before .getTables().size(), after.getTables().size()); } + + /* + * 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 static void waitForQuotaEnabled() throws Exception { UTIL.waitFor(60000, new Waiter.Predicate() { @@ -569,6 +624,7 @@ public class TestNamespaceAuditor { public static class MasterSyncObserver extends BaseMasterObserver { volatile CountDownLatch tableDeletionLatch; + static boolean throwExceptionInPreCreateTable; @Override public void preDeleteTable(ObserverContext ctx, @@ -582,7 +638,15 @@ public class TestNamespaceAuditor { 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 { // NOTE: We need a latch because admin is not sync, -- 1.9.5.msysgit.0