From aa20ffad8076ef9a9d2cad61032d771e44df2d81 Mon Sep 17 00:00:00 2001 From: "Apekshit(Appy) Sharma" Date: Wed, 8 Jul 2015 11:26:04 -0700 Subject: [PATCH] HBASE-14127 Refactor very non-trivial AssignmentManager.unassign() function. (Apekshit) --- .../hadoop/hbase/master/AssignmentManager.java | 59 ++++++++++------------ 1 file changed, 26 insertions(+), 33 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 f7f98fe..46178aa 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 @@ -1303,53 +1303,46 @@ public class AssignmentManager { * @param dest the destination server of the region */ public void unassign(HRegionInfo region, ServerName dest) { - // TODO: Method needs refactoring. Ugly buried returns throughout. Beware! LOG.debug("Starting unassign of " + region.getRegionNameAsString() + " (offlining), current state: " + regionStates.getRegionState(region)); String encodedName = region.getEncodedName(); - // Grab the state of this region and synchronize on it + // Grab the state of this region and synchronize on it. // We need a lock here as we're going to do a put later and we don't want multiple states - // creation + // creation. ReentrantLock lock = locker.acquireLock(encodedName); - RegionState state = regionStates.getRegionTransitionState(encodedName); + RegionState transitionState = regionStates.getRegionTransitionState(encodedName); try { - if (state == null || state.isFailedClose()) { - if (state == null) { - // Region is not in transition. - // We can unassign it only if it's not SPLIT/MERGED. - state = regionStates.getRegionState(encodedName); - if (state != null && state.isUnassignable()) { - LOG.info("Attempting to unassign " + state + ", ignored"); - // Offline region will be reassigned below - return; - } - if (state == null || state.getServerName() == null) { - // We don't know where the region is, offline it. - // No need to send CLOSE RPC - LOG.warn("Attempting to unassign a region not in RegionStates " - + region.getRegionNameAsString() + ", offlined"); - regionOffline(region); - return; - } + if (transitionState == null) { + // Region is not in transition. + // We can unassign it only if it's not SPLIT/MERGED. + RegionState currentState = regionStates.getRegionState(encodedName); + if (currentState != null && currentState.isUnassignable()) { + LOG.info("Ignoring unassign of region " + region.getRegionNameAsString() + " with state " + + currentState); + // Offline region will be reassigned in 'final' block. + } else if (currentState == null || currentState.getServerName() == null) { + // We don't know where the region is, offline it. + // No need to send CLOSE RPC + LOG.warn("Attempting to unassign a region not in RegionStates " + region + .getRegionNameAsString() + ", offlined"); + regionOffline(region); + } else { + RegionState state = regionStates.updateRegionState(region, State.PENDING_CLOSE); + unassign(region, state.getServerName(), dest); } - state = regionStates.updateRegionState( - region, State.PENDING_CLOSE); - } else if (state.isFailedOpen()) { + } else if (transitionState.isFailedClose()) { + RegionState state = regionStates.updateRegionState(region, State.PENDING_CLOSE); + unassign(region, state.getServerName(), dest); + } else if (transitionState.isFailedOpen()) { // The region is not open yet regionOffline(region); - return; } else { - LOG.debug("Attempting to unassign " + - region.getRegionNameAsString() + " but it is " + - "already in transition (" + state.getState()); - return; + LOG.debug("Attempting to unassign " + region.getRegionNameAsString() + " but it is " + + "already in transition (" + transitionState.getState()); } - - unassign(region, state.getServerName(), dest); } finally { lock.unlock(); - // Region is expected to be reassigned afterwards if (!replicasToClose.contains(region) && regionStates.isRegionInState(region, State.OFFLINE)) { -- 2.3.2 (Apple Git-55)