From fa298a9573f7dcc5eb1987818443791694f71ac4 Mon Sep 17 00:00:00 2001 From: jxiang Date: Sat, 18 Jan 2014 03:20:41 +0000 Subject: [PATCH] HBASE-10349 Table became unusable when master balanced its region after table was dropped git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1559311 13f79535-47bb-0310-9956-ffa450edef68 --- .../hbase/master/AssignmentManager.java | 22 +++++++-- .../hadoop/hbase/master/RegionStates.java | 32 ++++++++++++ .../master/handler/DeleteTableHandler.java | 6 ++- .../hbase/master/TestAssignmentManager.java | 22 +++++++++ .../TestAssignmentManagerOnCluster.java | 49 +++++++++++++++++++ 5 files changed, 126 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 02d4819d650..dc2f93a1fd3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -3144,9 +3144,6 @@ public class AssignmentManager extends ZooKeeperListener { * @param plan Plan to execute. */ public void balance(final RegionPlan plan) { - synchronized (this.regionPlans) { - this.regionPlans.put(plan.getRegionName(), plan); - } HRegionInfo hri = plan.getRegionInfo(); TableName tableName = hri.getTable(); if (zkTable.isDisablingOrDisabledTable(tableName)) { @@ -3154,7 +3151,24 @@ public class AssignmentManager extends ZooKeeperListener { + tableName); return; } - unassign(hri, false, plan.getDestination()); + + // Move the region only if it's assigned + String encodedName = hri.getEncodedName(); + ReentrantLock lock = locker.acquireLock(encodedName); + try { + if (!regionStates.isRegionOnline(hri)) { + RegionState state = regionStates.getRegionState(encodedName); + LOG.info("Ignored moving region not assigned: " + hri + ", " + + (state == null ? "not in region states" : state)); + return; + } + synchronized (this.regionPlans) { + this.regionPlans.put(plan.getRegionName(), plan); + } + unassign(hri, false, plan.getDestination()); + } finally { + lock.unlock(); + } } public void stop() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java index 7537115e8ab..eaa57fccf41 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java @@ -576,6 +576,23 @@ public class RegionStates { } } + /** + * A table is deleted. Remove its regions from all internal maps. + * We loop through all regions assuming we don't delete tables too much. + */ + public synchronized void tableDeleted(final TableName tableName) { + Set regionsToDelete = new HashSet(); + for (RegionState state: regionStates.values()) { + HRegionInfo region = state.getRegion(); + if (region.getTable().equals(tableName)) { + regionsToDelete.add(region); + } + } + for (HRegionInfo region: regionsToDelete) { + deleteRegion(region); + } + } + /** * Checking if a region was assigned to a server which is not online now. * If so, we should hold re-assign this region till SSH has split its hlogs. @@ -746,4 +763,19 @@ public class RegionStates { return null; } } + + /** + * Remove a region from all state maps. + */ + private void deleteRegion(final HRegionInfo hri) { + String encodedName = hri.getEncodedName(); + regionsInTransition.remove(encodedName); + regionStates.remove(encodedName); + lastAssignments.remove(encodedName); + ServerName sn = regionAssignments.remove(hri); + if (sn != null) { + Set regions = serverHoldings.get(sn); + regions.remove(hri); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java index 26812af33ef..31316ebbbf7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java @@ -117,7 +117,11 @@ public class DeleteTableHandler extends TableEventHandler { LOG.debug("Removing '" + tableName + "' descriptor."); this.masterServices.getTableDescriptors().remove(tableName); - // 7. If entry for this table in zk, and up in AssignmentManager, remove it. + // 7. Clean up regions of the table in RegionStates. + LOG.debug("Removing '" + tableName + "' from region states."); + states.tableDeleted(tableName); + + // 8. If entry for this table in zk, and up in AssignmentManager, remove it. LOG.debug("Marking '" + tableName + "' as deleted."); am.getZKTable().setDeletedTable(tableName); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java index 408509f68ed..050620e7a47 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java @@ -1339,4 +1339,26 @@ public class TestAssignmentManager { am.shutdown(); } } + + /** + * If a table is deleted, we should not be able to balance it anymore. + * Otherwise, the region will be brought back. + * @throws Exception + */ + @Test + public void testBalanceRegionOfDeletedTable() throws Exception { + CatalogTracker ct = Mockito.mock(CatalogTracker.class); + AssignmentManager am = new AssignmentManager(this.server, this.serverManager, + ct, balancer, null, null, master.getTableLockManager()); + RegionStates regionStates = am.getRegionStates(); + HRegionInfo hri = REGIONINFO; + regionStates.createRegionState(hri); + assertFalse(regionStates.isRegionInTransition(hri)); + RegionPlan plan = new RegionPlan(hri, SERVERNAME_A, SERVERNAME_B); + // Fake table is deleted + regionStates.tableDeleted(hri.getTable()); + am.balance(plan); + assertFalse("The region should not in transition", + regionStates.isRegionInTransition(hri)); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java index e81b8dda1cb..6f78a469334 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hbase.MiniHBaseCluster; import org.apache.hadoop.hbase.ServerLoad; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.UnknownRegionException; import org.apache.hadoop.hbase.catalog.MetaEditor; import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hbase.client.HTable; @@ -272,6 +273,54 @@ public class TestAssignmentManagerOnCluster { } } + /** + * If a table is deleted, we should not be able to move it anymore. + * Otherwise, the region will be brought back. + * @throws Exception + */ + @Test (timeout=50000) + public void testMoveRegionOfDeletedTable() throws Exception { + TableName table = + TableName.valueOf("testMoveRegionOfDeletedTable"); + HBaseAdmin admin = TEST_UTIL.getHBaseAdmin(); + try { + HRegionInfo hri = createTableAndGetOneRegion(table); + + HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); + AssignmentManager am = master.getAssignmentManager(); + RegionStates regionStates = am.getRegionStates(); + ServerName serverName = regionStates.getRegionServerOfRegion(hri); + ServerName destServerName = null; + for (int i = 0; i < 3; i++) { + HRegionServer destServer = TEST_UTIL.getHBaseCluster().getRegionServer(i); + if (!destServer.getServerName().equals(serverName)) { + destServerName = destServer.getServerName(); + break; + } + } + assertTrue(destServerName != null + && !destServerName.equals(serverName)); + + TEST_UTIL.deleteTable(table); + + try { + admin.move(hri.getEncodedNameAsBytes(), + Bytes.toBytes(destServerName.getServerName())); + fail("We should not find the region"); + } catch (IOException ioe) { + assertTrue(ioe instanceof UnknownRegionException); + } + + am.balance(new RegionPlan(hri, serverName, destServerName)); + assertFalse("The region should not be in transition", + regionStates.isRegionInTransition(hri)); + } finally { + if (admin.tableExists(table)) { + TEST_UTIL.deleteTable(table); + } + } + } + HRegionInfo createTableAndGetOneRegion( final TableName tableName) throws IOException, InterruptedException { HTableDescriptor desc = new HTableDescriptor(tableName);