From a4280c9ae8962c9b3b7eb8fb83f6d06741bf70fb Mon Sep 17 00:00:00 2001 From: "Apekshit(Appy) Sharma" Date: Tue, 21 Jul 2015 15:02:34 -0700 Subject: [PATCH] HBASE-14127 Refactor very non-trivial AssignmentManager.unassign() function. (Apekshit) --- .../hadoop/hbase/master/AssignmentManager.java | 38 ++++++++++------------ 1 file changed, 17 insertions(+), 21 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 eef22c4..6aa6b15 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 @@ -2501,7 +2501,6 @@ public class AssignmentManager extends ZooKeeperListener { * @param force if region should be closed even if already closing */ public void unassign(HRegionInfo region, boolean force, ServerName dest) { - // TODO: Method needs refactoring. Ugly buried returns throughout. Beware! LOG.debug("Starting unassign of " + region.getRegionNameAsString() + " (offlining), current state: " + regionStates.getRegionState(region)); @@ -2511,21 +2510,21 @@ public class AssignmentManager extends ZooKeeperListener { // We need a lock here as we're going to do a put later and we don't want multiple states // creation ReentrantLock lock = locker.acquireLock(encodedName); - RegionState state = regionStates.getRegionTransitionState(encodedName); + RegionState transitionState = regionStates.getRegionTransitionState(encodedName); boolean reassign = true; try { - if (state == null) { + if (transitionState == 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"); + RegionState currentState = regionStates.getRegionState(encodedName); + if (currentState != null && currentState.isUnassignable()) { + LOG.info("Attempting to unassign " + currentState + ", ignored"); // Offline region will be reassigned below return; } // Create the znode in CLOSING state try { - if (state == null || state.getServerName() == null) { + 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 " @@ -2535,7 +2534,7 @@ public class AssignmentManager extends ZooKeeperListener { } if (useZKForAssignment) { versionOfClosingNode = ZKAssign.createNodeClosing( - watcher, region, state.getServerName()); + watcher, region, currentState.getServerName()); if (versionOfClosingNode == -1) { LOG.info("Attempting to unassign " + region.getRegionNameAsString() + " but ZK closing node " @@ -2575,27 +2574,24 @@ public class AssignmentManager extends ZooKeeperListener { reassign = false; // heading out already return; } - state = regionStates.updateRegionState(region, State.PENDING_CLOSE); - } else if (state.isFailedOpen()) { + currentState = regionStates.updateRegionState(region, State.PENDING_CLOSE); + unassign(region, currentState, versionOfClosingNode, dest, useZKForAssignment, null); + } else if (transitionState.isFailedOpen()) { // The region is not open yet regionOffline(region); - return; - } else if (force && state.isPendingCloseOrClosing()) { - LOG.debug("Attempting to unassign " + region.getRegionNameAsString() + - " which is already " + state.getState() + - " but forcing to send a CLOSE RPC again "); + } else if (force && transitionState.isPendingCloseOrClosing()) { + LOG.debug("Attempting to unassign " + region.getRegionNameAsString() + " which is already " + + transitionState.getState() + " but forcing to send a CLOSE RPC again "); + RegionState state = transitionState; if (state.isFailedClose()) { state = regionStates.updateRegionState(region, State.PENDING_CLOSE); } state.updateTimestampToNow(); + unassign(region, state, versionOfClosingNode, dest, useZKForAssignment, null); } else { - LOG.debug("Attempting to unassign " + - region.getRegionNameAsString() + " but it is " + - "already in transition (" + state.getState() + ", force=" + force + ")"); - return; + LOG.debug("Attempting to unassign " + region.getRegionNameAsString() + " but it is " + + "already in transition (" + transitionState.getState() + ", force=" + force + ")"); } - - unassign(region, state, versionOfClosingNode, dest, useZKForAssignment, null); } finally { lock.unlock(); -- 2.3.2 (Apple Git-55)