### Eclipse Workspace Patch 1.0 #P hbase Index: src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java (revision 1335333) +++ src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java (working copy) @@ -284,10 +284,10 @@ // Iterate regions that were on this server and assign them if (hris != null) { for (Map.Entry e: hris.entrySet()) { + RegionState rit = this.services.getAssignmentManager().isRegionInTransition(e.getKey()); if (processDeadRegion(e.getKey(), e.getValue(), this.services.getAssignmentManager(), this.server.getCatalogTracker())) { - RegionState rit = this.services.getAssignmentManager().isRegionInTransition(e.getKey()); ServerName addressFromAM = this.services.getAssignmentManager() .getRegionServerOfRegion(e.getKey()); if (rit != null && !rit.isClosing() && !rit.isPendingClose()) { @@ -304,6 +304,20 @@ this.services.getAssignmentManager().assign(e.getKey(), true); } } + // If the table was partially disabled and the RS went down, we should clear the RIT + // and remove the node for the region. + // The rit that we use may be a stale in case when the table was in DISABLING state + // but though we did assign we will not be clearing the znode in CLOSING state. + // Doing this will have no harm. See HBASE-5927 + if (rit != null + && (rit.isClosing() || rit.isPendingClose()) + && this.services.getAssignmentManager().getZKTable() + .isDisablingOrDisabledTable(rit.getRegion().getTableNameAsString())) { + HRegionInfo hri = rit.getRegion(); + AssignmentManager am = this.services.getAssignmentManager(); + am.deleteClosingOrClosedNode(hri, false); + am.regionOffline(hri); + } } } } finally { Index: src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (revision 1335333) +++ src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (working copy) @@ -2024,7 +2024,7 @@ } } // delete the node. if no node exists need not bother. - deleteClosingOrClosedNode(region); + deleteClosingOrClosedNode(region, true); return; } try { @@ -2062,7 +2062,7 @@ synchronized (this.regions) { this.regions.remove(region); } - deleteClosingOrClosedNode(region); + deleteClosingOrClosedNode(region, true); } } // RS is already processing this region, only need to update the timestamp @@ -2077,7 +2077,7 @@ } } - private void deleteClosingOrClosedNode(HRegionInfo region) { + public void deleteClosingOrClosedNode(HRegionInfo region, boolean abort) { try { if (!ZKAssign.deleteNode(master.getZooKeeper(), region.getEncodedName(), EventHandler.EventType.M_ZK_REGION_CLOSING)) { @@ -2094,9 +2094,10 @@ LOG.debug("CLOSING/CLOSED node for the region " + region.getEncodedName() + " already deleted"); } catch (KeeperException ke) { - master.abort( - "Unexpected ZK exception deleting node CLOSING/CLOSED for the region " - + region.getEncodedName(), ke); + if (abort) { + master.abort("Unexpected ZK exception deleting node CLOSING/CLOSED for the region " + + region.getEncodedName(), ke); + } return; } } Index: src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (revision 1335333) +++ src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (working copy) @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.master; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -47,6 +48,8 @@ import org.apache.hadoop.hbase.executor.ExecutorService.ExecutorType; import org.apache.hadoop.hbase.executor.RegionTransitionData; import org.apache.hadoop.hbase.ipc.HRegionInterface; +import org.apache.hadoop.hbase.master.AssignmentManager.RegionState; +import org.apache.hadoop.hbase.master.AssignmentManager.RegionState.State; import org.apache.hadoop.hbase.master.handler.ServerShutdownHandler; import org.apache.hadoop.hbase.regionserver.RegionOpeningState; import org.apache.hadoop.hbase.util.Bytes; @@ -56,6 +59,7 @@ import org.apache.hadoop.hbase.zookeeper.ZKAssign; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.apache.hadoop.hbase.zookeeper.ZKTable.TableState; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.KeeperException.NodeExistsException; import org.junit.After; @@ -379,42 +383,68 @@ AssignmentManager am = new AssignmentManager(this.server, this.serverManager, ct, balancer, executor); try { - // Make sure our new AM gets callbacks; once registered, can't unregister. - // Thats ok because we make a new zk watcher for each test. - this.watcher.registerListenerFirst(am); + processServerShutdownHandler(ct, am); + } finally { + executor.shutdown(); + am.shutdown(); + // Clean up all znodes + ZKAssign.deleteAllNodes(this.watcher); + } + } - // Need to set up a fake scan of meta for the servershutdown handler - // Make an RS Interface implementation. Make it so a scanner can go against it. - HRegionInterface implementation = Mockito.mock(HRegionInterface.class); - // Get a meta row result that has region up on SERVERNAME_A - Result r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); - Mockito.when(implementation.openScanner((byte [])Mockito.any(), (Scan)Mockito.any())). - thenReturn(System.currentTimeMillis()); - // Return a good result first and then return null to indicate end of scan - Mockito.when(implementation.next(Mockito.anyLong(), Mockito.anyInt())). - thenReturn(new Result [] {r}, (Result [])null); + /** + * To test closed region handler to remove rit and delete corresponding znode if region in pending + * close or closing while processing shutdown of a region server.(HBASE-5927). + * @throws KeeperException + * @throws IOException + */ + @Test + public void testSSHWhenDisableTableInProgress() + throws KeeperException, IOException { + testCaseWithPartiallyDisabledState(TableState.DISABLING); + testCaseWithPartiallyDisabledState(TableState.DISABLED); + } - // Get a connection w/ mocked up common methods. - HConnection connection = - HConnectionTestingUtility.getMockedConnectionAndDecorate(HTU.getConfiguration(), - implementation, SERVERNAME_B, REGIONINFO); + private void testCaseWithPartiallyDisabledState(TableState state) throws KeeperException, IOException, NodeExistsException { + // Create and startup an executor. This is used by AssignmentManager + // handling zk callbacks. + ExecutorService executor = startupMasterExecutor("testSSHWhenDisableTableInProgress"); - // Make it so we can get a catalogtracker from servermanager.. .needed - // down in guts of server shutdown handler. - Mockito.when(ct.getConnection()).thenReturn(connection); - Mockito.when(this.server.getCatalogTracker()).thenReturn(ct); + // We need a mocked catalog tracker. + CatalogTracker ct = Mockito.mock(CatalogTracker.class); + LoadBalancer balancer = LoadBalancerFactory.getLoadBalancer(server.getConfiguration()); + // Create an AM. + AssignmentManager am = new AssignmentManager(this.server, this.serverManager, ct, balancer, + executor); + // adding region to regions and servers maps. + am.regionOnline(REGIONINFO, SERVERNAME_A); + // adding region in pending close. + am.regionsInTransition.put(REGIONINFO.getEncodedName(), new RegionState(REGIONINFO, + State.PENDING_CLOSE)); - // Now make a server shutdown handler instance and invoke process. - // Have it that SERVERNAME_A died. - DeadServer deadServers = new DeadServer(); - deadServers.add(SERVERNAME_A); - // I need a services instance that will return the AM - MasterServices services = Mockito.mock(MasterServices.class); - Mockito.when(services.getAssignmentManager()).thenReturn(am); - ServerShutdownHandler handler = new ServerShutdownHandler(this.server, - services, deadServers, SERVERNAME_A, false); - handler.process(); - // The region in r will have been assigned. It'll be up in zk as unassigned. + if (state == TableState.DISABLING) { + am.getZKTable().setDisablingTable(REGIONINFO.getTableNameAsString()); + } else { + am.getZKTable().setDisabledTable(REGIONINFO.getTableNameAsString()); + } + + RegionTransitionData data = new RegionTransitionData(EventType.M_ZK_REGION_CLOSING, + REGIONINFO.getRegionName(), SERVERNAME_A); + String node = ZKAssign.getNodeName(this.watcher, REGIONINFO.getEncodedName()); + // create znode in M_ZK_REGION_CLOSING state. + ZKUtil.createAndWatch(this.watcher, node, data.getBytes()); + + try { + processServerShutdownHandler(ct, am); + // check znode deleted or not. + // In both cases the znode should be deleted. + assertTrue("The znode should be deleted.",ZKUtil.checkExists(this.watcher, node) == -1); + // check whether in rit or not. In the DISABLING case also the below assert will be true + // but the piece of code added for HBASE-5927 will not do that. + if (state == TableState.DISABLED) { + assertTrue("Region state of region in pending close should be removed from rit.", + am.regionsInTransition.isEmpty()); + } } finally { executor.shutdown(); am.shutdown(); @@ -423,6 +453,46 @@ } } + private void processServerShutdownHandler(CatalogTracker ct, AssignmentManager am) + throws IOException { + // Make sure our new AM gets callbacks; once registered, can't unregister. + // Thats ok because we make a new zk watcher for each test. + this.watcher.registerListenerFirst(am); + + // Need to set up a fake scan of meta for the servershutdown handler + // Make an RS Interface implementation. Make it so a scanner can go against it. + HRegionInterface implementation = Mockito.mock(HRegionInterface.class); + // Get a meta row result that has region up on SERVERNAME_A + Result r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); + Mockito.when(implementation.openScanner((byte [])Mockito.any(), (Scan)Mockito.any())). + thenReturn(System.currentTimeMillis()); + // Return a good result first and then return null to indicate end of scan + Mockito.when(implementation.next(Mockito.anyLong(), Mockito.anyInt())). + thenReturn(new Result [] {r}, (Result [])null); + + // Get a connection w/ mocked up common methods. + HConnection connection = + HConnectionTestingUtility.getMockedConnectionAndDecorate(HTU.getConfiguration(), + implementation, SERVERNAME_B, REGIONINFO); + + // Make it so we can get a catalogtracker from servermanager.. .needed + // down in guts of server shutdown handler. + Mockito.when(ct.getConnection()).thenReturn(connection); + Mockito.when(this.server.getCatalogTracker()).thenReturn(ct); + + // Now make a server shutdown handler instance and invoke process. + // Have it that SERVERNAME_A died. + DeadServer deadServers = new DeadServer(); + deadServers.add(SERVERNAME_A); + // I need a services instance that will return the AM + MasterServices services = Mockito.mock(MasterServices.class); + Mockito.when(services.getAssignmentManager()).thenReturn(am); + ServerShutdownHandler handler = new ServerShutdownHandler(this.server, + services, deadServers, SERVERNAME_A, false); + handler.process(); + // The region in r will have been assigned. It'll be up in zk as unassigned. + } + /** * @param sn ServerName to use making startcode and server in meta * @param hri Region to serialize into HRegionInfo