Index: src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (revision 1345623) +++ src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (working copy) @@ -163,6 +163,10 @@ private final SortedMap regions = new TreeMap(); + // To maintain regions of dead servers from region plan. + private final NavigableMap deadServerRegionsFromRegionPlan = + new ConcurrentSkipListMap(); + private final ExecutorService executorService; //Thread pool executor service for timeout monitor @@ -281,6 +285,14 @@ } /** + * + * @return dead server regions from region plan. + */ + public NavigableMap getDeadServerRegionsFromRegionPlan(){ + return this.deadServerRegionsFromRegionPlan; + } + + /** * Set the list of regions that will be reopened * because of an update in table schema * @@ -823,7 +835,10 @@ data.getStamp(), data.getOrigin()); // When there are more than one region server a new RS is selected as the // destination and the same is updated in the regionplan. (HBASE-5546) - getRegionPlan(regionState, sn, true); + RegionPlan regionPlan = getRegionPlan(regionState, sn, true); + if (regionPlan == RegionPlan.UNUSABLE_PLAN) { + break; + } this.executorService.submit(new ClosedRegionHandler(master, this, regionState.getRegion())); break; @@ -1601,11 +1616,14 @@ return; } RegionPlan plan = getRegionPlan(state, forceNewPlan); - if (plan == null) { + if (plan == RegionPlan.NO_SERVERS_TO_ASSIGN) { LOG.debug("Unable to determine a plan to assign " + state); this.timeoutMonitor.setAllRegionServersOffline(true); return; // Should get reassigned later when RIT times out. } + if (plan == RegionPlan.UNUSABLE_PLAN) { + return; + } try { LOG.debug("Assigning region " + state.getRegion().getRegionNameAsString() + " to " + plan.getDestination().toString()); @@ -1663,11 +1681,14 @@ // Transition back to OFFLINE state.update(RegionState.State.OFFLINE); // Force a new plan and reassign. Will return null if no servers. - if (getRegionPlan(state, plan.getDestination(), true) == null) { + RegionPlan newPlan = getRegionPlan(state, plan.getDestination(), true); + if (newPlan == RegionPlan.NO_SERVERS_TO_ASSIGN) { this.timeoutMonitor.setAllRegionServersOffline(true); LOG.warn("Unable to find a viable location to assign region " + state.getRegion().getRegionNameAsString()); return; + } else if (newPlan == RegionPlan.UNUSABLE_PLAN) { + return; } } } @@ -1784,7 +1805,7 @@ * @param forceNewPlan If true, then if an existing plan exists, a new plan * will be generated. * @return Plan for passed state (If none currently, it creates one or - * if no servers to assign, it returns null). + * if no servers to assign, it returns RegionPlan.NO_SERVERS_TO_ASSIGN). */ RegionPlan getRegionPlan(final RegionState state, final ServerName serverToExclude, final boolean forceNewPlan) { @@ -1793,7 +1814,6 @@ final List servers = this.serverManager.getOnlineServersList(); final List drainingServers = this.serverManager.getDrainingServersList(); - if (serverToExclude != null) servers.remove(serverToExclude); // Loop through the draining server list and remove them from the server @@ -1805,14 +1825,11 @@ servers.remove(server); } } - // Remove the deadNotExpired servers from the server list. removeDeadNotExpiredServers(servers); + if (servers.isEmpty()) return RegionPlan.NO_SERVERS_TO_ASSIGN; - - if (servers.isEmpty()) return null; - RegionPlan randomPlan = null; boolean newPlan = false; RegionPlan existingPlan = null; @@ -1835,20 +1852,42 @@ .randomAssignment(servers)); this.regionPlans.put(encodedName, randomPlan); } + + if (serverToExclude != null) { + RegionsOnDeadServer regionsOnDeadServer = this.deadServerRegionsFromRegionPlan + .get(serverToExclude); + if (regionsOnDeadServer != null + && regionsOnDeadServer.getRegionsFromRegionPlansForServer(). + contains(state.getRegion())) { + if (newPlan) { + this.regionPlans.remove(randomPlan.getRegionName()); + LOG + .info("Server shutdown handler already in progress for the region " + + randomPlan.getRegionName()); + randomPlan = RegionPlan.UNUSABLE_PLAN; + } else { + this.regionPlans.remove(existingPlan.getRegionName()); + LOG + .info("Server shutdown handler already in progress for the region " + + existingPlan.getRegionName()); + existingPlan = RegionPlan.UNUSABLE_PLAN; + } + } + } } if (newPlan) { LOG.debug("No previous transition plan was found (or we are ignoring " + - "an existing plan) for " + state.getRegion().getRegionNameAsString() + - " so generated a random one; " + randomPlan + "; " + - serverManager.countOfRegionServers() + - " (online=" + serverManager.getOnlineServers().size() + - ", available=" + servers.size() + ") available servers"); - return randomPlan; - } + "an existing plan) for " + state.getRegion().getRegionNameAsString() + + " so generated a random one; " + randomPlan + "; " + + serverManager.countOfRegionServers() + + " (online=" + serverManager.getOnlineServers().size() + + ", available=" + servers.size() + ") available servers"); + return randomPlan; + } LOG.debug("Using pre-existing plan for region " + - state.getRegion().getRegionNameAsString() + "; plan=" + existingPlan); - return existingPlan; + state.getRegion().getRegionNameAsString() + "; plan=" + existingPlan); + return existingPlan; } /** @@ -3034,11 +3073,14 @@ /** * Process shutdown server removing any assignments. * @param sn Server that went down. - * @return list of regions in transition on this server + * @return list of regions in transition and region plans on this server */ - public List processServerShutdown(final ServerName sn) { + public RegionsOnDeadServer processServerShutdown(final ServerName sn) { + RegionsOnDeadServer regionsOnDeadServer = new RegionsOnDeadServer(); + Set regionsFromRegionPlansForServer = new ConcurrentSkipListSet(); // Clean out any existing assignment plans for this server synchronized (this.regionPlans) { + this.deadServerRegionsFromRegionPlan.put(sn, regionsOnDeadServer); for (Iterator > i = this.regionPlans.entrySet().iterator(); i.hasNext();) { Map.Entry e = i.next(); @@ -3046,9 +3088,11 @@ // The name will be null if the region is planned for a random assign. if (otherSn != null && otherSn.equals(sn)) { // Use iterator's remove else we'll get CME + regionsFromRegionPlansForServer.add(e.getValue().getRegionInfo()); i.remove(); } } + regionsOnDeadServer.setRegionsFromRegionPlansForServer(regionsFromRegionPlansForServer); } // TODO: Do we want to sync on RIT here? // Remove this server from map of servers to regions, and remove all regions @@ -3059,7 +3103,8 @@ Set assignedRegions = this.servers.remove(sn); if (assignedRegions == null || assignedRegions.isEmpty()) { // No regions on this server, we are done, return empty list of RITs - return rits; + regionsOnDeadServer.setRegionsInTransition(rits); + return regionsOnDeadServer; } deadRegions = new TreeSet(assignedRegions); for (HRegionInfo region : deadRegions) { @@ -3076,7 +3121,8 @@ } } } - return rits; + regionsOnDeadServer.setRegionsInTransition(rits); + return regionsOnDeadServer; } /** @@ -3395,4 +3441,56 @@ this.master.abort(errorMsg, e); } } + + /** + * + * @param hri + * @return whether region is online or not. + */ + public boolean isRegionOnline(HRegionInfo hri) { + ServerName sn = null; + synchronized (this.regions) { + sn = this.regions.get(hri); + if (sn == null) { + return false; + } + if (this.isServerOnline(sn)) { + return true; + } + // Remove the assignment mapping for sn. + Set hriSet = this.servers.get(sn); + if (hriSet != null) { + hriSet.remove(hri); + } + this.regions.remove(hri); + return false; + } + } + + /** + * Store the related regions on a dead server, used by processServerShutdown. + */ + public static class RegionsOnDeadServer { + // The regions which being processed on this dead server. + private Set regionsFromRegionPlansForServer = null; + private List regionsInTransition = null; + + public Set getRegionsFromRegionPlansForServer() { + return regionsFromRegionPlansForServer; + } + + public void setRegionsFromRegionPlansForServer( + Set regionsFromRegionPlansForServer) { + this.regionsFromRegionPlansForServer = regionsFromRegionPlansForServer; + } + + public List getRegionsInTransition() { + return regionsInTransition; + } + + public void setRegionsInTransition(List regionsInTransition) { + this.regionsInTransition = regionsInTransition; + } + } + } Index: src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java (revision 1345623) +++ src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java (working copy) @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.NavigableMap; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -37,6 +38,7 @@ import org.apache.hadoop.hbase.executor.EventHandler; import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.AssignmentManager.RegionState; +import org.apache.hadoop.hbase.master.AssignmentManager.RegionsOnDeadServer; import org.apache.hadoop.hbase.master.DeadServer; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.ServerManager; @@ -168,6 +170,8 @@ @Override public void process() throws IOException { final ServerName serverName = this.serverName; + Set regionsFromRegionPlansForServer = null; + List regionsInTransition = null; try { try { if (this.shouldSplitHlog) { @@ -229,9 +233,10 @@ // doing after log splitting. Could do some states before -- OPENING? // OFFLINE? -- and then others after like CLOSING that depend on log // splitting. - List regionsInTransition = - this.services.getAssignmentManager(). - processServerShutdown(this.serverName); + RegionsOnDeadServer regionsOnDeadServer = this.services.getAssignmentManager() + .processServerShutdown(this.serverName); + regionsFromRegionPlansForServer = regionsOnDeadServer.getRegionsFromRegionPlansForServer(); + regionsInTransition = regionsOnDeadServer.getRegionsInTransition(); // Wait on meta to come online; we need it to progress. // TODO: Best way to hold strictly here? We should build this retry logic @@ -290,19 +295,19 @@ this.server.getCatalogTracker())) { ServerName addressFromAM = this.services.getAssignmentManager() .getRegionServerOfRegion(e.getKey()); - if (rit != null && !rit.isClosing() && !rit.isPendingClose() && !rit.isSplitting()) { + if (rit != null && !rit.isClosing() && !rit.isPendingClose() && !rit.isSplitting() + && !regionsFromRegionPlansForServer.contains(rit.getRegion())) { // Skip regions that were in transition unless CLOSING or - // PENDING_CLOSE + // PENDING_CLOSE or splitting LOG.info("Skip assigning region " + rit.toString()); - } else if (addressFromAM != null - && !addressFromAM.equals(this.serverName)) { - LOG.debug("Skip assigning region " - + e.getKey().getRegionNameAsString() - + " because it has been opened in " - + addressFromAM.getServerName()); - } else { - this.services.getAssignmentManager().assign(e.getKey(), true); - } + } else if (addressFromAM != null && !addressFromAM.equals(this.serverName)) { + LOG.debug("Skip assigning region " + e.getKey().getRegionNameAsString() + + " because it has been opened in " + addressFromAM.getServerName()); + regionsFromRegionPlansForServer.remove(e.getKey()); + } else { + this.services.getAssignmentManager().assign(e.getKey(), true); + regionsFromRegionPlansForServer.remove(e.getKey()); + } } else if (rit != null && (rit.isSplitting() || rit.isSplit())) { // This will happen when the RS went down and the call back for the SPLIITING or SPLIT // has not yet happened for node Deleted event. In that case if the region was actually split @@ -312,6 +317,7 @@ HRegionInfo region = rit.getRegion(); AssignmentManager am = this.services.getAssignmentManager(); am.regionOffline(region); + regionsFromRegionPlansForServer.remove(e.getKey()); } // If the table was partially disabled and the RS went down, we should clear the RIT // and remove the node for the region. @@ -326,10 +332,31 @@ AssignmentManager am = this.services.getAssignmentManager(); am.deleteClosingOrClosedNode(hri); am.regionOffline(hri); + regionsFromRegionPlansForServer.remove(hri); } } } + + int reassignedRegions = 0; + for (HRegionInfo hri : regionsFromRegionPlansForServer) { + if (!this.services.getAssignmentManager().isRegionOnline(hri)) { + this.services.getAssignmentManager().assign(hri, true); + reassignedRegions++; + } + } + regionsFromRegionPlansForServer.clear(); + LOG.info(reassignedRegions + " regions which were planned to open on " + this.serverName + + " have been re-assigned."); + } finally { + if (regionsFromRegionPlansForServer != null) { + regionsFromRegionPlansForServer.clear(); + } + if (regionsInTransition != null) { + regionsInTransition.clear(); + } + this.services.getAssignmentManager().getDeadServerRegionsFromRegionPlan() + .remove(this.serverName); this.deadServers.finish(serverName); } LOG.info("Finished processing of shutdown of " + serverName); Index: src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java (revision 1345623) +++ src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java (working copy) @@ -31,6 +31,10 @@ * information and not the source/dest server info. */ public class RegionPlan implements Comparable { + // the following singleton signifies that the plan is not usable + static final RegionPlan UNUSABLE_PLAN = new RegionPlan(null, null, null); + // the following singleton signifies that there is no region server to assign region + static final RegionPlan NO_SERVERS_TO_ASSIGN = new RegionPlan(null, null, null); private final HRegionInfo hri; private final ServerName source; private ServerName dest; @@ -98,8 +102,9 @@ @Override public String toString() { + if (this == UNUSABLE_PLAN || this == NO_SERVERS_TO_ASSIGN) return ""; return "hri=" + this.hri.getRegionNameAsString() + ", src=" + (this.source == null? "": this.source.toString()) + ", dest=" + (this.dest == null? "": this.dest.toString()); - } + } } Index: src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (revision 1345623) +++ src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (working copy) @@ -26,8 +26,10 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -53,6 +55,7 @@ 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.AssignmentManager.RegionsOnDeadServer; import org.apache.hadoop.hbase.master.handler.ServerShutdownHandler; import org.apache.hadoop.hbase.regionserver.RegionOpeningState; import org.apache.hadoop.hbase.util.Bytes; @@ -376,9 +379,10 @@ * Run a simple server shutdown handler. * @throws KeeperException * @throws IOException + * @throws ServiceException */ @Test - public void testShutdownHandler() throws KeeperException, IOException { + public void testShutdownHandler() throws KeeperException, IOException, ServiceException { // Create and startup an executor. This is used by AssignmentManager // handling zk callbacks. ExecutorService executor = startupMasterExecutor("testShutdownHandler"); @@ -391,7 +395,7 @@ AssignmentManager am = new AssignmentManager(this.server, this.serverManager, ct, balancer, executor); try { - processServerShutdownHandler(ct, am, false); + processServerShutdownHandler(ct, am, false,null); } finally { executor.shutdown(); am.shutdown(); @@ -405,10 +409,11 @@ * close or closing while processing shutdown of a region server.(HBASE-5927). * @throws KeeperException * @throws IOException + * @throws ServiceException */ @Test - public void testSSHWhenDisableTableInProgress() - throws KeeperException, IOException { + public void testSSHWhenDisableTableInProgress() throws KeeperException, IOException, + ServiceException { testCaseWithPartiallyDisabledState(TableState.DISABLING); testCaseWithPartiallyDisabledState(TableState.DISABLED); } @@ -431,7 +436,7 @@ } private void testCaseWithSplitRegionPartial(boolean regionSplitDone) throws KeeperException, IOException, - NodeExistsException, InterruptedException { + NodeExistsException, InterruptedException, ServiceException { // Create and startup an executor. This is used by AssignmentManager // handling zk callbacks. ExecutorService executor = startupMasterExecutor("testSSHWhenSplitRegionInProgress"); @@ -455,7 +460,7 @@ try { - processServerShutdownHandler(ct, am, regionSplitDone); + processServerShutdownHandler(ct, am, regionSplitDone, null); // check znode deleted or not. // In both cases the znode should be deleted. @@ -479,7 +484,8 @@ } } - private void testCaseWithPartiallyDisabledState(TableState state) throws KeeperException, IOException, NodeExistsException { + private void testCaseWithPartiallyDisabledState(TableState state) throws KeeperException, + IOException, NodeExistsException, ServiceException { // Create and startup an executor. This is used by AssignmentManager // handling zk callbacks. ExecutorService executor = startupMasterExecutor("testSSHWhenDisableTableInProgress"); @@ -509,7 +515,7 @@ ZKUtil.createAndWatch(this.watcher, node, data.getBytes()); try { - processServerShutdownHandler(ct, am, false); + processServerShutdownHandler(ct, am, false, null); // 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); @@ -527,8 +533,115 @@ } } - private void processServerShutdownHandler(CatalogTracker ct, AssignmentManager am, boolean splitRegion) - throws IOException { + /** + * + * Test verifies if region assignment through balancer is skipped when SSH processing a region. + * See HBASE-5816. + */ + @Test + public void testAssignThroughBalancerOrFailedOpenAndSSHInParallel() throws IOException, + KeeperException, ServiceException, InterruptedException { + try { + this.server.getConfiguration().setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, + MockedLoadBalancer.class, LoadBalancer.class); + CatalogTracker ct = Mockito.mock(CatalogTracker.class); + AssignmentManagerWithExtrasForTesting am = setUpMockedAssignmentManager(this.server, + this.serverManager); + // Boolean variable used for waiting until randomAssignment is called and + // new + // plan is generated. + AtomicBoolean gate = new AtomicBoolean(false); + if (balancer instanceof MockedLoadBalancer) { + ((MockedLoadBalancer) balancer).setGateVariable(gate); + } + ZKAssign.deleteAllNodes(this.watcher); + ZKAssign.createNodeOffline(this.watcher, REGIONINFO, SERVERNAME_A); + int v = ZKAssign.getVersion(this.watcher, REGIONINFO); + ZKAssign.transitionNode(this.watcher, REGIONINFO, SERVERNAME_A, + EventType.M_ZK_REGION_OFFLINE, EventType.RS_ZK_REGION_FAILED_OPEN, v); + String path = ZKAssign.getNodeName(this.watcher, REGIONINFO.getEncodedName()); + RegionState state = new RegionState(REGIONINFO, State.OPENING, System.currentTimeMillis(), + SERVERNAME_B); + am.regionsInTransition.put(REGIONINFO.getEncodedName(), state); + // a dummy plan inserted into the regionPlans. This plan is cleared and + // new one is formed + am.regionPlans.put(REGIONINFO.getEncodedName(), + new RegionPlan(REGIONINFO, null, SERVERNAME_B)); + List serverList = new ArrayList(2); + serverList.add(SERVERNAME_B); + Mockito.when(this.serverManager.getOnlineServersList()).thenReturn(serverList); + // In SSH we populate regions from region plans of dead server. See HBASE-5396 + addRegionToDeadServerRegions(am); + am.nodeDataChanged(path); + am.regionPlans.put(REGIONINFO.getEncodedName(), + new RegionPlan(REGIONINFO, null, SERVERNAME_B)); + processServerShutdownHandler(ct, am, false, SERVERNAME_B); + + // new region plan may take some time to get updated after random + // assignment is called and gate is set to true. + RegionPlan newRegionPlan = am.regionPlans.get(REGIONINFO.getEncodedName()); + while (newRegionPlan == null) { + Thread.sleep(10); + newRegionPlan = am.regionPlans.get(REGIONINFO.getEncodedName()); + } + // The new region plan should not be used because region assignment is in progress in SSH. + assertTrue("Assign should be invoked.", am.assignInvoked); + + } finally { + this.server.getConfiguration().setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, + DefaultLoadBalancer.class, LoadBalancer.class); + // Clean up all znodes + ZKAssign.deleteAllNodes(this.watcher); + + } + } + + private void addRegionToDeadServerRegions(AssignmentManagerWithExtrasForTesting am) { + Set regionsSet = new HashSet(1); + regionsSet.add(REGIONINFO); + RegionsOnDeadServer rds = new RegionsOnDeadServer(); + rds.setRegionsFromRegionPlansForServer(regionsSet); + am.getDeadServerRegionsFromRegionPlan().put(SERVERNAME_A, rds); + } + + + /** + * When region in transition if region server opening the region gone down then region assignment + * taking long time(Waiting for timeout monitor to trigger assign). HBASE-5396(HBASE-6060) fixes this + * scenario. This test case verifies whether SSH calling assign for the region in transition or not. + * + * @throws KeeperException + * @throws IOException + * @throws ServiceException + */ + @Test + public void testSSHWhenSourceRSandDestRSInRegionPlanGoneDown() throws KeeperException, IOException, + ServiceException { + // We need a mocked catalog tracker. + CatalogTracker ct = Mockito.mock(CatalogTracker.class); + // Create an AM. + AssignmentManagerWithExtrasForTesting am = + setUpMockedAssignmentManager(this.server, this.serverManager); + // adding region in pending open. + am.regionsInTransition.put(REGIONINFO.getEncodedName(), new RegionState(REGIONINFO, + State.PENDING_OPEN)); + // adding region plan + am.regionPlans.put(REGIONINFO.getEncodedName(), new RegionPlan(REGIONINFO, SERVERNAME_A, SERVERNAME_B)); + am.getZKTable().setEnabledTable(REGIONINFO.getTableNameAsString()); + + try { + processServerShutdownHandler(ct, am, false, SERVERNAME_A); + processServerShutdownHandler(ct, am, false, SERVERNAME_B); + assertTrue("Assign should be invoked.", am.assignInvoked); + } finally { + am.regionsInTransition.remove(REGIONINFO.getEncodedName()); + am.regionPlans.remove(REGIONINFO.getEncodedName()); + } + + } + + private void processServerShutdownHandler(CatalogTracker ct, AssignmentManager am, + boolean splitRegion, ServerName sn) throws IOException, ServiceException { // 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); @@ -537,11 +650,20 @@ HRegionInterface implementation = Mockito.mock(HRegionInterface.class); // Get a meta row result that has region up on SERVERNAME_A Result r = null; - if (splitRegion) { - r = getMetaTableRowResultAsSplitRegion(REGIONINFO, SERVERNAME_A); + if (sn == null) { + if (splitRegion) { + r = getMetaTableRowResultAsSplitRegion(REGIONINFO, SERVERNAME_A); + } else { + r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); + } } else { - r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); + if (sn.equals(SERVERNAME_A)) { + r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); + } else if (sn.equals(SERVERNAME_B)) { + r = new Result(new KeyValue[0]); + } } + 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 @@ -565,8 +687,12 @@ // 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); + ServerShutdownHandler handler = null; + if(sn != null){ + handler = new ServerShutdownHandler(this.server, services, deadServers, sn, false); + } else{ + 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. }