HBASE-9748 Address outstanding comments raised for HBASE-9696

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1535771 13f79535-47bb-0310-9956-ffa450edef68

hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java

@@ -202,24 +202,34 @@ public class RegionState implements org.apache.hadoop.io.Writable {
     return serverName != null && serverName.equals(sn);
   }
 
-  // Is a region in a state ready to go offline
+  /**
+   * Check if a region state can transition to offline
+   */
   public boolean isReadyToOffline() {
     return isMerged() || isSplit() || isOffline()
       || isSplittingNew() || isMergingNew();
   }
 
-  // Is a region in a state ready to go online
+  /**
+   * Check if a region state can transition to online
+   */
   public boolean isReadyToOnline() {
     return isOpened() || isSplittingNew() || isMergingNew();
   }
 
-  // Is a region in a state not in transition but not unassignable
+  /**
-  public boolean isNotUnassignableNotInTransition() {
+   * Check if a region state is one of offline states that
-    return isNotUnassignableNotInTransition(state);
+   * can't transition to pending_close/closing (unassign/offline)
+   */
+  public boolean isUnassignable() {
+    return isUnassignable(state);
   }
 
-  // Check if a state is not in transition, but not unassignable
+  /**
-  public static boolean isNotUnassignableNotInTransition(State state) {
+   * Check if a region state is one of offline states that
+   * can't transition to pending_close/closing (unassign/offline)
+   */
+  public static boolean isUnassignable(State state) {
     return state == State.MERGED || state == State.SPLIT || state == State.OFFLINE
       || state == State.SPLITTING_NEW || state == State.MERGING_NEW;
   }

hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

@@ -259,13 +259,15 @@ public class ZKAssign {
    *
    * @param zkw zk reference
    * @param encodedRegionName opened region to be deleted from zk
+   * @param sn the expected region transition target server name
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteOpenedNode(ZooKeeperWatcher zkw,
-      String encodedRegionName)
+      String encodedRegionName, ServerName sn)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_OPENED);
+    return deleteNode(zkw, encodedRegionName,
+      EventType.RS_ZK_REGION_OPENED, sn);
   }
 
   /**
@@ -284,13 +286,15 @@ public class ZKAssign {
    *
    * @param zkw zk reference
    * @param encodedRegionName closed region to be deleted from zk
+   * @param sn the expected region transition target server name
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteOfflineNode(ZooKeeperWatcher zkw,
-      String encodedRegionName)
+      String encodedRegionName, ServerName sn)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_OFFLINE);
+    return deleteNode(zkw, encodedRegionName,
+      EventType.M_ZK_REGION_OFFLINE, sn);
   }
 
   /**
@@ -310,13 +314,15 @@ public class ZKAssign {
    *
    * @param zkw zk reference
    * @param encodedRegionName closed region to be deleted from zk
+   * @param sn the expected region transition target server name
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteClosedNode(ZooKeeperWatcher zkw,
-      String encodedRegionName)
+      String encodedRegionName, ServerName sn)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_CLOSED);
+    return deleteNode(zkw, encodedRegionName,
+      EventType.RS_ZK_REGION_CLOSED, sn);
   }
 
   /**
@@ -336,14 +342,16 @@ public class ZKAssign {
    *
    * @param zkw zk reference
    * @param region closing region to be deleted from zk
+   * @param sn the expected region transition target server name
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteClosingNode(ZooKeeperWatcher zkw,
-      HRegionInfo region)
+      HRegionInfo region, ServerName sn)
   throws KeeperException, KeeperException.NoNodeException {
     String encodedRegionName = region.getEncodedName();
-    return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_CLOSING);
+    return deleteNode(zkw, encodedRegionName,
+      EventType.M_ZK_REGION_CLOSING, sn);
   }
 
   /**
@@ -364,13 +372,14 @@ public class ZKAssign {
    * @param zkw zk reference
    * @param encodedRegionName region to be deleted from zk
    * @param expectedState state region must be in for delete to complete
+   * @param sn the expected region transition target server name
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName,
-      EventType expectedState)
+      EventType expectedState, ServerName sn)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, encodedRegionName, expectedState, -1);
+    return deleteNode(zkw, encodedRegionName, expectedState, sn, -1);
   }
 
   /**
@@ -399,6 +408,37 @@ public class ZKAssign {
    */
   public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName,
       EventType expectedState, int expectedVersion)
+  throws KeeperException, KeeperException.NoNodeException {
+    return deleteNode(zkw, encodedRegionName, expectedState, null, expectedVersion);
+  }
+
+  /**
+   * Deletes an existing unassigned node that is in the specified state for the
+   * specified region.
+   *
+   * <p>If a node does not already exist for this region, a
+   * {@link NoNodeException} will be thrown.
+   *
+   * <p>No watcher is set whether this succeeds or not.
+   *
+   * <p>Returns false if the node was not in the proper state but did exist.
+   *
+   * <p>This method is used when a region finishes opening/closing.
+   * The Master acknowledges completion
+   * of the specified regions transition to being closed/opened.
+   *
+   * @param zkw zk reference
+   * @param encodedRegionName region to be deleted from zk
+   * @param expectedState state region must be in for delete to complete
+   * @param sn the expected region transition target server name
+   * @param expectedVersion of the znode that is to be deleted.
+   *        If expectedVersion need not be compared while deleting the znode
+   *        pass -1
+   * @throws KeeperException if unexpected zookeeper exception
+   * @throws KeeperException.NoNodeException if node does not exist
+   */
+  public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName,
+      EventType expectedState, ServerName serverName, int expectedVersion)
   throws KeeperException, KeeperException.NoNodeException {
     if (LOG.isTraceEnabled()) {
     	LOG.trace(zkw.prefix("Deleting existing unassigned " +
@@ -419,6 +459,12 @@ public class ZKAssign {
         expectedState + " state but node is in " + et + " state"));
       return false;
     }
+    // Verify the server transition happens on is not changed
+    if (serverName != null && !rt.getServerName().equals(serverName)) {
+      LOG.warn(zkw.prefix("Attempting to delete unassigned node " + encodedRegionName
+        + " with target " + serverName + " but node has " + rt.getServerName()));
+      return false;
+    }
     if (expectedVersion != -1
         && stat.getVersion() != expectedVersion) {
       LOG.warn("The node " + encodedRegionName + " we are trying to delete is not" +

hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java

@@ -715,7 +715,7 @@ public class IntegrationTestBigLinkedList extends IntegrationTestBase {
           String keyString = it.next().getName();
           byte[] key = Bytes.toBytes(keyString);
           HRegionLocation loc = conn.relocateRegion(tableName, key);
-          LOG.error("undefined row " + keyString + " region " + loc);
+          LOG.error("undefined row " + keyString + ", " + loc);
         }
         g = counters.getGroup("unref");
         it = g.iterator();
@@ -723,7 +723,7 @@ public class IntegrationTestBigLinkedList extends IntegrationTestBase {
           String keyString = it.next().getName();
           byte[] key = Bytes.toBytes(keyString);
           HRegionLocation loc = conn.relocateRegion(tableName, key);
-          LOG.error("unreferred row " + keyString + " region " + loc);
+          LOG.error("unreferred row " + keyString + ", " + loc);
         }
       }
       return success;

hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

@@ -566,10 +566,11 @@ public class AssignmentManager extends ZooKeeperListener {
       }
       HRegionInfo hri = regionInfo;
       if (hri == null) {
-        // Get the region from region states map/meta. However, we
+        // The region info is not passed in. We will try to find the region
-        // may still can't get it, for example, for online region merge,
+        // from region states map/meta based on the encoded region name. But we
-        // the znode uses the new region to be created, which may not in meta
+        // may not be able to find it. This is valid for online merge that
-        // yet if the merging is still going on during the master recovery.
+        // the region may have not been created if the merge is not completed.
+        // Therefore, it is not in meta at master recovery time.
         hri = regionStates.getRegionInfo(rt.getRegionName());
         EventType et = rt.getEventType();
         if (hri == null && et != EventType.RS_ZK_REGION_MERGING
@@ -601,9 +602,12 @@ public class AssignmentManager extends ZooKeeperListener {
     final byte[] regionName = rt.getRegionName();
     final String encodedName = HRegionInfo.encodeRegionName(regionName);
     final String prettyPrintedRegionName = HRegionInfo.prettyPrint(encodedName);
-    LOG.info("Processing " + prettyPrintedRegionName + " in state " + et);
+    LOG.info("Processing " + prettyPrintedRegionName + " in state: " + et);
 
     if (regionStates.isRegionInTransition(encodedName)) {
+      LOG.info("Processed region " + prettyPrintedRegionName + " in state: "
+        + et + ", does nothing since the region is already in transition "
+        + regionStates.getRegionTransitionState(encodedName));
       // Just return
       return;
     }
@@ -696,29 +700,29 @@ public class AssignmentManager extends ZooKeeperListener {
           // Splitting region should be online. We could have skipped it during
           // user region rebuilding since we may consider the split is completed.
           // Put it in SPLITTING state to avoid complications.
+          regionStates.updateRegionState(regionInfo, State.OPEN, sn);
           regionStates.regionOnline(regionInfo, sn);
           regionStates.updateRegionState(rt, State.SPLITTING);
         }
         if (!handleRegionSplitting(
             rt, encodedName, prettyPrintedRegionName, sn)) {
-          deleteSplittingNode(encodedName);
+          deleteSplittingNode(encodedName, sn);
         }
-        LOG.info("Processed region " + prettyPrintedRegionName
-          + " in state : " + et);
         break;
       case RS_ZK_REQUEST_REGION_MERGE:
       case RS_ZK_REGION_MERGING:
       case RS_ZK_REGION_MERGED:
         if (!handleRegionMerging(
             rt, encodedName, prettyPrintedRegionName, sn)) {
-          deleteMergingNode(encodedName);
+          deleteMergingNode(encodedName, sn);
         }
-        LOG.info("Processed region " + prettyPrintedRegionName
-          + " in state : " + et);
         break;
       default:
         throw new IllegalStateException("Received region in state:" + et + " is not valid.");
     }
+    LOG.info("Processed region " + prettyPrintedRegionName + " in state "
+      + et + ", on " + (serverManager.isServerOnline(sn) ? "" : "dead ")
+      + "server: " + sn);
   }
 
   /**
@@ -836,7 +840,7 @@ public class AssignmentManager extends ZooKeeperListener {
       case RS_ZK_REGION_SPLIT:
         if (!handleRegionSplitting(
             rt, encodedName, prettyPrintedRegionName, sn)) {
-          deleteSplittingNode(encodedName);
+          deleteSplittingNode(encodedName, sn);
         }
         break;
 
@@ -847,7 +851,7 @@ public class AssignmentManager extends ZooKeeperListener {
         // However, the two merging regions are not new. They should be in state for merging.
         if (!handleRegionMerging(
             rt, encodedName, prettyPrintedRegionName, sn)) {
-          deleteMergingNode(encodedName);
+          deleteMergingNode(encodedName, sn);
         }
         break;
 
@@ -1388,17 +1392,9 @@ public class AssignmentManager extends ZooKeeperListener {
     LOG.debug("Table being disabled so deleting ZK node and removing from " +
       "regions in transition, skipping assignment of region " +
         regionInfo.getRegionNameAsString());
-    try {
+    String encodedName = regionInfo.getEncodedName();
-      if (!ZKAssign.deleteClosedNode(watcher, regionInfo.getEncodedName())) {
+    deleteNodeInStates(encodedName, "closed", null,
-        // Could also be in OFFLINE mode
+      EventType.RS_ZK_REGION_CLOSED, EventType.M_ZK_REGION_OFFLINE);
-        ZKAssign.deleteOfflineNode(watcher, regionInfo.getEncodedName());
-      }
-    } catch (KeeperException.NoNodeException nne) {
-      LOG.debug("Tried to delete closed node for " + regionInfo + " but it " +
-          "does not exist so just offlining");
-    } catch (KeeperException e) {
-      this.server.abort("Error deleting CLOSED node in ZK", e);
-    }
     regionOffline(regionInfo);
   }
 
@@ -1678,9 +1674,11 @@ public class AssignmentManager extends ZooKeeperListener {
       }
       // ClosedRegionhandler can remove the server from this.regions
       if (!serverManager.isServerOnline(server)) {
+        LOG.debug("Offline " + region.getRegionNameAsString()
+          + ", no need to unassign since it's on a dead server: " + server);
         if (transitionInZK) {
           // delete the node. if no node exists need not bother.
-          deleteClosingOrClosedNode(region);
+          deleteClosingOrClosedNode(region, server);
         }
         if (state != null) {
           regionOffline(region);
@@ -1711,8 +1709,10 @@ public class AssignmentManager extends ZooKeeperListener {
         }
         if (t instanceof NotServingRegionException
             || t instanceof RegionServerStoppedException) {
+          LOG.debug("Offline " + region.getRegionNameAsString()
+            + ", it's not any more on " + server, t);
           if (transitionInZK) {
-            deleteClosingOrClosedNode(region);
+            deleteClosingOrClosedNode(region, server);
           }
           if (state != null) {
             regionOffline(region);
@@ -2076,20 +2076,8 @@ public class AssignmentManager extends ZooKeeperListener {
     // already enabled.
     LOG.debug("ALREADY_OPENED " + region.getRegionNameAsString()
       + " to " + sn);
-    String encodedRegionName = region.getEncodedName();
+    String encodedName = region.getEncodedName();
-    try {
+    deleteNodeInStates(encodedName, "offline", sn, EventType.M_ZK_REGION_OFFLINE);
-      ZKAssign.deleteOfflineNode(watcher, encodedRegionName);
-    } catch (KeeperException.NoNodeException e) {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("The unassigned node " + encodedRegionName
-            + " does not exist.");
-      }
-    } catch (KeeperException e) {
-      server.abort(
-          "Error deleting OFFLINED node in ZK for transition ZK node ("
-              + encodedRegionName + ")", e);
-    }
-
     regionStates.regionOnline(region, sn);
   }
 
@@ -2220,37 +2208,6 @@ public class AssignmentManager extends ZooKeeperListener {
     return existingPlan;
   }
 
-  /**
-   * Unassign the list of regions. Configuration knobs:
-   * hbase.bulk.waitbetween.reopen indicates the number of milliseconds to
-   * wait before unassigning another region from this region server
-   *
-   * @param regions
-   * @throws InterruptedException
-   */
-  public void unassign(List<HRegionInfo> regions) {
-    int waitTime = this.server.getConfiguration().getInt(
-        "hbase.bulk.waitbetween.reopen", 0);
-    for (HRegionInfo region : regions) {
-      if (regionStates.isRegionInTransition(region))
-        continue;
-      unassign(region, false);
-      while (regionStates.isRegionInTransition(region)) {
-        try {
-          Thread.sleep(10);
-        } catch (InterruptedException e) {
-          // Do nothing, continue
-        }
-      }
-      if (waitTime > 0)
-        try {
-          Thread.sleep(waitTime);
-        } catch (InterruptedException e) {
-          // Do nothing, continue
-        }
-    }
-  }
-
   /**
    * Unassigns the specified region.
    * <p>
@@ -2301,7 +2258,7 @@ public class AssignmentManager extends ZooKeeperListener {
         // Region is not in transition.
         // We can unassign it only if it's not SPLIT/MERGED.
         state = regionStates.getRegionState(encodedName);
-        if (state != null && state.isNotUnassignableNotInTransition()) {
+        if (state != null && state.isUnassignable()) {
           LOG.info("Attempting to unassign " + state + ", ignored");
           // Offline region will be reassigned below
           return;
@@ -2394,9 +2351,9 @@ public class AssignmentManager extends ZooKeeperListener {
   /**
    * @param region regioninfo of znode to be deleted.
    */
-  public void deleteClosingOrClosedNode(HRegionInfo region) {
+  public void deleteClosingOrClosedNode(HRegionInfo region, ServerName sn) {
-    String regionName = region.getEncodedName();
+    String encodedName = region.getEncodedName();
-    deleteNodeInStates(regionName, "closing", EventType.M_ZK_REGION_CLOSING,
+    deleteNodeInStates(encodedName, "closing", sn, EventType.M_ZK_REGION_CLOSING,
       EventType.RS_ZK_REGION_CLOSED);
   }
 
@@ -3175,7 +3132,6 @@ public class AssignmentManager extends ZooKeeperListener {
             server.abort("Unexpected ZK exception deleting node " + hri, ke);
           }
           if (zkTable.isDisablingOrDisabledTable(hri.getTable())) {
-            regionStates.updateRegionState(hri, State.OFFLINE);
             regionStates.regionOffline(hri);
             it.remove();
             continue;
@@ -3266,32 +3222,34 @@ public class AssignmentManager extends ZooKeeperListener {
     return true;
   }
 
-  private boolean deleteNodeInStates(
+  private boolean deleteNodeInStates(String encodedName,
-      String regionName, String desc, EventType... types) {
+      String desc, ServerName sn, EventType... types) {
     try {
       for (EventType et: types) {
-        if (ZKAssign.deleteNode(watcher, regionName, et)) {
+        if (ZKAssign.deleteNode(watcher, encodedName, et, sn)) {
           return true;
         }
       }
       LOG.info("Failed to delete the " + desc + " node for "
-        + regionName + ". The node type may not match");
+        + encodedName + ". The node type may not match");
     } catch (NoNodeException e) {
-      LOG.debug("The " + desc + " node for " + regionName + " already deleted");
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("The " + desc + " node for " + encodedName + " already deleted");
+      }
     } catch (KeeperException ke) {
       server.abort("Unexpected ZK exception deleting " + desc
-        + " node for the region " + regionName, ke);
+        + " node for the region " + encodedName, ke);
     }
     return false;
   }
 
-  private void deleteMergingNode(String encodedName) {
+  private void deleteMergingNode(String encodedName, ServerName sn) {
-    deleteNodeInStates(encodedName, "merging", EventType.RS_ZK_REGION_MERGING,
+    deleteNodeInStates(encodedName, "merging", sn, EventType.RS_ZK_REGION_MERGING,
       EventType.RS_ZK_REQUEST_REGION_MERGE, EventType.RS_ZK_REGION_MERGED);
   }
 
-  private void deleteSplittingNode(String encodedName) {
+  private void deleteSplittingNode(String encodedName, ServerName sn) {
-    deleteNodeInStates(encodedName, "splitting", EventType.RS_ZK_REGION_SPLITTING,
+    deleteNodeInStates(encodedName, "splitting", sn, EventType.RS_ZK_REGION_SPLITTING,
       EventType.RS_ZK_REQUEST_REGION_SPLIT, EventType.RS_ZK_REGION_SPLIT);
   }
 
@@ -3359,21 +3317,16 @@ public class AssignmentManager extends ZooKeeperListener {
     }
 
     synchronized (regionStates) {
-      if (regionStates.getRegionState(p) == null) {
-        regionStates.createRegionState(p);
-      }
       regionStates.updateRegionState(hri_a, State.MERGING);
       regionStates.updateRegionState(hri_b, State.MERGING);
+      regionStates.updateRegionState(p, State.MERGING_NEW, sn);
 
       if (et != EventType.RS_ZK_REGION_MERGED) {
-        regionStates.updateRegionState(p, State.MERGING_NEW, sn);;
         regionStates.regionOffline(p, State.MERGING_NEW);
         this.mergingRegions.put(encodedName,
           new PairOfSameType<HRegionInfo>(hri_a, hri_b));
       } else {
         this.mergingRegions.remove(encodedName);
-        regionStates.updateRegionState(hri_a, State.MERGED);
-        regionStates.updateRegionState(hri_b, State.MERGED);
         regionOffline(hri_a, State.MERGED);
         regionOffline(hri_b, State.MERGED);
         regionOnline(p, sn);
@@ -3388,8 +3341,8 @@ public class AssignmentManager extends ZooKeeperListener {
         while (!successful) {
           // It's possible that the RS tickles in between the reading of the
           // znode and the deleting, so it's safe to retry.
-          successful = ZKAssign.deleteNode(
+          successful = ZKAssign.deleteNode(watcher, encodedName,
-            watcher, encodedName, EventType.RS_ZK_REGION_MERGED);
+            EventType.RS_ZK_REGION_MERGED, sn);
         }
       } catch (KeeperException e) {
         if (e instanceof NoNodeException) {
@@ -3486,13 +3439,6 @@ public class AssignmentManager extends ZooKeeperListener {
     }
 
     synchronized (regionStates) {
-      if (regionStates.getRegionState(hri_a) == null) {
-        regionStates.createRegionState(hri_a);
-      }
-      if (regionStates.getRegionState(hri_b) == null) {
-        regionStates.createRegionState(hri_b);
-      }
-
       regionStates.updateRegionState(hri_a, State.SPLITTING_NEW, sn);
       regionStates.updateRegionState(hri_b, State.SPLITTING_NEW, sn);
       regionStates.regionOffline(hri_a, State.SPLITTING_NEW);
@@ -3507,7 +3453,6 @@ public class AssignmentManager extends ZooKeeperListener {
       }
 
       if (et == EventType.RS_ZK_REGION_SPLIT) {
-        regionStates.updateRegionState(p, State.SPLIT);
         regionOffline(p, State.SPLIT);
         regionOnline(hri_a, sn);
         regionOnline(hri_b, sn);
@@ -3522,8 +3467,8 @@ public class AssignmentManager extends ZooKeeperListener {
         while (!successful) {
           // It's possible that the RS tickles in between the reading of the
           // znode and the deleting, so it's safe to retry.
-          successful = ZKAssign.deleteNode(
+          successful = ZKAssign.deleteNode(watcher, encodedName,
-            watcher, encodedName, EventType.RS_ZK_REGION_SPLIT);
+            EventType.RS_ZK_REGION_SPLIT, sn);
         }
       } catch (KeeperException e) {
         if (e instanceof NoNodeException) {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/BulkReOpen.java

@@ -67,7 +67,12 @@ public class BulkReOpen extends BulkAssigner {
       assignmentManager.addPlans(plans);
       pool.execute(new Runnable() {
         public void run() {
-          assignmentManager.unassign(hris);
+          try {
+            unassign(hris);
+          } catch (Throwable t) {
+            LOG.warn("Failed bulking re-open " + hris.size()
+              + " region(s)", t);
+          }
         }
       });
     }
@@ -97,4 +102,35 @@ public class BulkReOpen extends BulkAssigner {
   public boolean bulkReOpen() throws InterruptedException, IOException {
     return bulkAssign();
   }
+
+  /**
+   * Unassign the list of regions. Configuration knobs:
+   * hbase.bulk.waitbetween.reopen indicates the number of milliseconds to
+   * wait before unassigning another region from this region server
+   *
+   * @param regions
+   * @throws InterruptedException
+   */
+  private void unassign(
+      List<HRegionInfo> regions) throws InterruptedException {
+    int waitTime = this.server.getConfiguration().getInt(
+        "hbase.bulk.waitbetween.reopen", 0);
+    RegionStates regionStates = assignmentManager.getRegionStates();
+    for (HRegionInfo region : regions) {
+      if (server.isStopped()) {
+        return;
+      }
+      if (regionStates.isRegionInTransition(region)) {
+        continue;
+      }
+      assignmentManager.unassign(region, false);
+      while (regionStates.isRegionInTransition(region)
+          && !server.isStopped()) {
+        regionStates.waitForUpdate(100);
+      }
+      if (waitTime > 0 && !server.isStopped()) {
+        Thread.sleep(waitTime);
+      }
+    }
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -87,6 +87,7 @@ import org.apache.hadoop.hbase.ipc.RpcServer;
 import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface;
 import org.apache.hadoop.hbase.ipc.RpcServerInterface;
 import org.apache.hadoop.hbase.ipc.ServerRpcController;
+import org.apache.hadoop.hbase.master.RegionState.State;
 import org.apache.hadoop.hbase.master.balancer.BalancerChore;
 import org.apache.hadoop.hbase.master.balancer.ClusterStatusChore;
 import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory;
@@ -990,17 +991,18 @@ MasterServices, Server {
     status.setStatus("Assigning hbase:meta region");
     ServerName logReplayFailedMetaServer = null;
 
-    assignmentManager.getRegionStates().createRegionState(HRegionInfo.FIRST_META_REGIONINFO);
+    RegionStates regionStates = assignmentManager.getRegionStates();
+    regionStates.createRegionState(HRegionInfo.FIRST_META_REGIONINFO);
     boolean rit = this.assignmentManager
       .processRegionInTransitionAndBlockUntilAssigned(HRegionInfo.FIRST_META_REGIONINFO);
     boolean metaRegionLocation = this.catalogTracker.verifyMetaRegionLocation(timeout);
+    ServerName currentMetaServer = this.catalogTracker.getMetaLocation();
     if (!metaRegionLocation) {
       // Meta location is not verified. It should be in transition, or offline.
       // We will wait for it to be assigned in enableSSHandWaitForMeta below.
       assigned++;
       if (!rit) {
         // Assign meta since not already in transition
-        ServerName currentMetaServer = this.catalogTracker.getMetaLocation();
         if (currentMetaServer != null) {
           if (expireIfOnline(currentMetaServer)) {
             splitMetaLogBeforeAssignment(currentMetaServer);
@@ -1013,8 +1015,10 @@ MasterServices, Server {
       }
     } else {
       // Region already assigned. We didn't assign it. Add to in-memory state.
-      this.assignmentManager.regionOnline(HRegionInfo.FIRST_META_REGIONINFO,
+      regionStates.updateRegionState(
-        this.catalogTracker.getMetaLocation());
+        HRegionInfo.FIRST_META_REGIONINFO, State.OPEN, currentMetaServer);
+      this.assignmentManager.regionOnline(
+        HRegionInfo.FIRST_META_REGIONINFO, currentMetaServer);
     }
 
     enableMeta(TableName.META_TABLE_NAME);

hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java

@@ -158,8 +158,8 @@ public class RegionStates {
   /**
    * @return True if specified region in transition.
    */
-  public synchronized boolean isRegionInTransition(final String regionName) {
+  public synchronized boolean isRegionInTransition(final String encodedName) {
-    return regionsInTransition.containsKey(regionName);
+    return regionsInTransition.containsKey(encodedName);
   }
 
   /**
@@ -197,8 +197,8 @@ public class RegionStates {
    * @return True if specified region is in one of the specified states.
    */
   public synchronized boolean isRegionInState(
-      final String regionName, final State... states) {
+      final String encodedName, final State... states) {
-    RegionState regionState = getRegionState(regionName);
+    RegionState regionState = getRegionState(encodedName);
     State s = regionState != null ? regionState.getState() : null;
     for (State state: states) {
       if (s == state) return true;
@@ -226,8 +226,8 @@ public class RegionStates {
    * Get region transition state
    */
   public synchronized RegionState
-      getRegionTransitionState(final String regionName) {
+      getRegionTransitionState(final String encodedName) {
-    return regionsInTransition.get(regionName);
+    return regionsInTransition.get(encodedName);
   }
 
   /**
@@ -250,14 +250,14 @@ public class RegionStates {
    */
   public synchronized RegionState createRegionState(final HRegionInfo hri) {
     State newState = (hri.isOffline() && hri.isSplit()) ? State.SPLIT : State.OFFLINE;
-    String regionName = hri.getEncodedName();
+    String encodedName = hri.getEncodedName();
-    RegionState regionState = regionStates.get(regionName);
+    RegionState regionState = regionStates.get(encodedName);
     if (regionState != null) {
       LOG.warn("Tried to create a state for a region already in RegionStates, "
         + "used existing: " + regionState + ", ignored new: " + newState);
     } else {
       regionState = new RegionState(hri, newState);
-      regionStates.put(regionName, regionState);
+      regionStates.put(encodedName, regionState);
     }
     return regionState;
   }
@@ -313,28 +313,28 @@ public class RegionStates {
         + " on " + serverName + ", set to " + state);
     }
 
-    String regionName = hri.getEncodedName();
+    String encodedName = hri.getEncodedName();
     RegionState regionState = new RegionState(
       hri, state, System.currentTimeMillis(), newServerName);
-    RegionState oldState = regionStates.put(regionName, regionState);
+    RegionState oldState = regionStates.put(encodedName, regionState);
     if (oldState == null || oldState.getState() != regionState.getState()) {
       LOG.info("Transitioned " + oldState + " to " + regionState);
     }
     if (newServerName != null || (
         state != State.PENDING_CLOSE && state != State.CLOSING)) {
-      regionsInTransition.put(regionName, regionState);
+      regionsInTransition.put(encodedName, regionState);
     }
 
     // For these states, region should be properly closed.
     // There should be no log splitting issue.
     if ((state == State.CLOSED || state == State.MERGED
-        || state == State.SPLIT) && lastAssignments.containsKey(regionName)) {
+        || state == State.SPLIT) && lastAssignments.containsKey(encodedName)) {
       ServerName oldServerName = oldState == null ? null : oldState.getServerName();
-      ServerName last = lastAssignments.get(regionName);
+      ServerName last = lastAssignments.get(encodedName);
       if (last.equals(oldServerName)) {
-        lastAssignments.remove(regionName);
+        lastAssignments.remove(encodedName);
       } else {
-        LOG.warn(regionName + " moved to " + state + " on "
+        LOG.warn(encodedName + " moved to " + state + " on "
           + oldServerName + ", expected " + last);
       }
     }
@@ -361,8 +361,8 @@ public class RegionStates {
       return;
     }
 
-    String regionName = hri.getEncodedName();
+    String encodedName = hri.getEncodedName();
-    RegionState oldState = regionStates.get(regionName);
+    RegionState oldState = regionStates.get(encodedName);
     if (oldState == null) {
       LOG.warn("Online region not in RegionStates: " + hri.getShortNameToLog());
     } else {
@@ -374,9 +374,9 @@ public class RegionStates {
       }
     }
     updateRegionState(hri, State.OPEN, serverName);
-    regionsInTransition.remove(regionName);
+    regionsInTransition.remove(encodedName);
 
-    lastAssignments.put(regionName, serverName);
+    lastAssignments.put(encodedName, serverName);
     ServerName oldServerName = regionAssignments.put(hri, serverName);
     if (!serverName.equals(oldServerName)) {
       LOG.info("Onlined " + hri.getShortNameToLog() + " on " + serverName);
@@ -449,30 +449,13 @@ public class RegionStates {
   public synchronized void regionOffline(
       final HRegionInfo hri, final State expectedState) {
     Preconditions.checkArgument(expectedState == null
-      || RegionState.isNotUnassignableNotInTransition(expectedState),
+      || RegionState.isUnassignable(expectedState),
-        "Offlined region should be in state OFFLINE/SPLIT/MERGED/"
+        "Offlined region should not be " + expectedState);
-          + "SPLITTING_NEW/MERGING_NEW instead of " + expectedState);
+    String encodedName = hri.getEncodedName();
-    String regionName = hri.getEncodedName();
+    State newState =
-    RegionState oldState = regionStates.get(regionName);
+      expectedState == null ? State.OFFLINE : expectedState;
-    if (oldState == null) {
-      LOG.warn("Offline region not in RegionStates: " + hri.getShortNameToLog());
-    } else if (LOG.isDebugEnabled()) {
-      ServerName sn = oldState.getServerName();
-      if (!oldState.isReadyToOffline()) {
-        LOG.debug("Offline " + hri.getShortNameToLog() + " with current state="
-          + oldState.getState() + ", expected state=OFFLINE/SPLIT/"
-          + "MERGED/SPLITTING_NEW/MERGING_NEW");
-      }
-      if (sn != null && oldState.isOffline()) {
-        LOG.debug("Offline " + hri.getShortNameToLog()
-          + " with current state=OFFLINE, assigned to server: "
-          + sn + ", expected null");
-      }
-    }
-    State newState = expectedState;
-    if (newState == null) newState = State.OFFLINE;
     updateRegionState(hri, newState);
-    regionsInTransition.remove(regionName);
+    regionsInTransition.remove(encodedName);
 
     ServerName oldServerName = regionAssignments.remove(hri);
     if (oldServerName != null) {
@@ -518,7 +501,6 @@ public class RegionStates {
     }
 
     for (HRegionInfo hri : regionsToOffline) {
-      updateRegionState(hri, State.OFFLINE);
       regionOffline(hri);
     }
 
@@ -604,8 +586,8 @@ public class RegionStates {
    * think it's online falsely. Therefore if a server is online, we still
    * need to confirm it reachable and having the expected start code.
    */
-  synchronized boolean wasRegionOnDeadServer(final String regionName) {
+  synchronized boolean wasRegionOnDeadServer(final String encodedName) {
-    ServerName server = lastAssignments.get(regionName);
+    ServerName server = lastAssignments.get(encodedName);
     return isServerDeadAndNotProcessed(server);
   }
 
@@ -639,8 +621,8 @@ public class RegionStates {
    * Get the last region server a region was on for purpose of re-assignment,
    * i.e. should the re-assignment be held back till log split is done?
    */
-  synchronized ServerName getLastRegionServerOfRegion(final String regionName) {
+  synchronized ServerName getLastRegionServerOfRegion(final String encodedName) {
-    return lastAssignments.get(regionName);
+    return lastAssignments.get(encodedName);
   }
 
   synchronized void setLastRegionServerOfRegions(
@@ -729,8 +711,8 @@ public class RegionStates {
     return regionStates.get(hri.getEncodedName());
   }
 
-  protected synchronized RegionState getRegionState(final String regionName) {
+  protected synchronized RegionState getRegionState(final String encodedName) {
-    return regionStates.get(regionName);
+    return regionStates.get(encodedName);
   }
 
   /**

hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java

@@ -39,7 +39,6 @@ import org.apache.hadoop.hbase.master.MasterFileSystem;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.RegionStates;
 import org.apache.hadoop.hbase.master.RegionState.State;
-import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Threads;
 import org.apache.zookeeper.KeeperException;
 

hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java

@@ -266,7 +266,7 @@ public class ServerShutdownHandler extends EventHandler {
                 // but though we did assign we will not be clearing the znode in CLOSING state.
                 // Doing this will have no harm. See HBASE-5927
                 regionStates.updateRegionState(hri, State.OFFLINE);
-                am.deleteClosingOrClosedNode(hri);
+                am.deleteClosingOrClosedNode(hri, rit.getServerName());
                 am.offlineDisabledRegion(hri);
               } else {
                 LOG.warn("THIS SHOULD NOT HAPPEN: unexpected region in transition "

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java

@@ -725,12 +725,12 @@ public class RegionMergeTransaction {
     try {
       // Only delete if its in expected state; could have been hijacked.
       if (!ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(),
-          RS_ZK_REQUEST_REGION_MERGE)) {
+          RS_ZK_REQUEST_REGION_MERGE, server.getServerName())) {
         ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(),
-          RS_ZK_REGION_MERGING);
+          RS_ZK_REGION_MERGING, server.getServerName());
       }
     } catch (KeeperException.NoNodeException e) {
-      LOG.warn("Failed cleanup zk node of " + hri.getRegionNameAsString(), e);
+      LOG.info("Failed cleanup zk node of " + hri.getRegionNameAsString(), e);
     } catch (KeeperException e) {
       server.abort("Failed cleanup zk node of " + hri.getRegionNameAsString(),e);
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

@@ -871,12 +871,12 @@ public class SplitTransaction {
     try {
       // Only delete if its in expected state; could have been hijacked.
       if (!ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(),
-          RS_ZK_REQUEST_REGION_SPLIT)) {
+          RS_ZK_REQUEST_REGION_SPLIT, server.getServerName())) {
         ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(),
-          RS_ZK_REGION_SPLITTING);
+          RS_ZK_REGION_SPLITTING, server.getServerName());
       }
     } catch (KeeperException.NoNodeException e) {
-      LOG.warn("Failed cleanup zk node of " + hri.getRegionNameAsString(), e);
+      LOG.info("Failed cleanup zk node of " + hri.getRegionNameAsString(), e);
     } catch (KeeperException e) {
       server.abort("Failed cleanup of " + hri.getRegionNameAsString(), e);
     }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

@@ -682,7 +682,7 @@ public class TestAssignmentManager {
       // First make sure my mock up basically works.  Unassign a region.
       unassign(am, SERVERNAME_A, hri);
       // This delete will fail if the previous unassign did wrong thing.
-      ZKAssign.deleteClosingNode(this.watcher, hri);
+      ZKAssign.deleteClosingNode(this.watcher, hri, SERVERNAME_A);
       // Now put a SPLITTING region in the way.  I don't have to assert it
       // go put in place.  This method puts it in place then asserts it still
       // owns it by moving state from SPLITTING to SPLITTING.

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java

@@ -776,7 +776,7 @@ public class TestMasterFailover {
       byte [] bytes = ZKAssign.getData(zkw, region.getEncodedName());
       RegionTransition rt = RegionTransition.parseFrom(bytes);
       if (rt != null && rt.getEventType().equals(EventType.RS_ZK_REGION_OPENED)) {
-        ZKAssign.deleteOpenedNode(zkw, region.getEncodedName());
+        ZKAssign.deleteOpenedNode(zkw, region.getEncodedName(), rt.getServerName());
         LOG.debug("DELETED " + rt);
         break;
       }
@@ -794,7 +794,7 @@ public class TestMasterFailover {
       byte [] bytes = ZKAssign.getData(zkw, region.getEncodedName());
       RegionTransition rt = RegionTransition.parseFrom(bytes);
       if (rt != null && rt.getEventType().equals(EventType.RS_ZK_REGION_OPENED)) {
-        ZKAssign.deleteOpenedNode(zkw, region.getEncodedName());
+        ZKAssign.deleteOpenedNode(zkw, region.getEncodedName(), rt.getServerName());
         break;
       }
       Thread.sleep(100);

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java

@@ -123,7 +123,8 @@ public class TestRegionServerNoMaster {
     Assert.assertTrue(getRS().getRegion(regionName).isAvailable());
 
     Assert.assertTrue(
-        ZKAssign.deleteOpenedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName()));
+      ZKAssign.deleteOpenedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(),
+        getRS().getServerName()));
   }
 
 
@@ -194,7 +195,8 @@ public class TestRegionServerNoMaster {
 
     checkRegionIsClosed();
 
-    ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName());
+    ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(),
+      getRS().getServerName());
 
     reopenRegion();
   }
@@ -274,7 +276,8 @@ public class TestRegionServerNoMaster {
     checkRegionIsClosed();
 
     Assert.assertTrue(
-        ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName())
+      ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(),
+        getRS().getServerName())
     );
 
     reopenRegion();

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java

@@ -419,8 +419,9 @@ public class TestSplitTransactionOnCluster {
       int regionCount = ProtobufUtil.getOnlineRegions(server).size();
       // Insert into zk a blocking znode, a znode of same name as region
       // so it gets in way of our splitting.
+      ServerName fakedServer = new ServerName("any.old.server", 1234, -1);
       ZKAssign.createNodeClosing(TESTING_UTIL.getZooKeeperWatcher(),
-        hri, new ServerName("any.old.server", 1234, -1));
+        hri, fakedServer);
       // Now try splitting.... should fail.  And each should successfully
       // rollback.
       this.admin.split(hri.getRegionNameAsString());
@@ -432,7 +433,8 @@ public class TestSplitTransactionOnCluster {
         assertEquals(regionCount, ProtobufUtil.getOnlineRegions(server).size());
       }
       // Now clear the zknode
-      ZKAssign.deleteClosingNode(TESTING_UTIL.getZooKeeperWatcher(), hri);
+      ZKAssign.deleteClosingNode(TESTING_UTIL.getZooKeeperWatcher(),
+        hri, fakedServer);
       // Now try splitting and it should work.
       split(hri, server, regionCount);
       // Get daughters

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java

@@ -211,7 +211,8 @@ public class TestCloseRegionHandler {
        // This parse is not used?
        RegionTransition.parseFrom(ZKAssign.getData(server.getZooKeeper(), hri.getEncodedName()));
        // delete the node, which is what Master do after the region is opened
-       ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), EventType.RS_ZK_REGION_OPENED);
+       ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(),
+         EventType.RS_ZK_REGION_OPENED, server.getServerName());
      }
 
 }