HBASE-921 region close and open processed out of order; makes for disagreement between master and regionserver on region state

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

CHANGES.txt

@@ -7,6 +7,8 @@ Release 0.19.0 - Unreleased
                Jim Kellerman)
    HBASE-852   Cannot scan all families in a row with a LIMIT, STARTROW, etc.
                (Izaak Rubin via Stack)
+   HBASE-921   region close and open processed out of order; makes for 
+               disagreement between master and regionserver on region state
 
   BUG FIXES
    HBASE-891   HRS.validateValuesLength throws IOE, gets caught in the retries

src/java/org/apache/hadoop/hbase/master/ProcessRegionClose.java

@@ -33,19 +33,21 @@ import org.apache.hadoop.hbase.HRegionInfo;
  * necessary.
  */
 class ProcessRegionClose extends ProcessRegionStatusChange {
-  protected final  boolean offlineRegion;
+  protected final boolean offlineRegion;
+  protected final boolean reassignRegion;
 
   /**
   * @param master
   * @param regionInfo Region to operate on
   * @param offlineRegion if true, set the region to offline in meta
-  * delete the region files from disk.
+  * @param reassignRegion if true, region is to be reassigned
   */
   public ProcessRegionClose(HMaster master, HRegionInfo regionInfo, 
-   boolean offlineRegion) {
+      boolean offlineRegion, boolean reassignRegion) {
 
    super(master, regionInfo);
    this.offlineRegion = offlineRegion;
+   this.reassignRegion = reassignRegion;
   }
 
   @Override
@@ -56,31 +58,34 @@ class ProcessRegionClose extends ProcessRegionStatusChange {
 
   @Override
   protected boolean process() throws IOException {
-    Boolean result =
-      new RetryableMetaOperation<Boolean>(this.metaRegion, this.master) {
-        public Boolean call() throws IOException {
-          LOG.info("region closed: " + regionInfo.getRegionNameAsString());
+    Boolean result = null;
+    if (offlineRegion) {
+      result =
+        new RetryableMetaOperation<Boolean>(this.metaRegion, this.master) {
+          public Boolean call() throws IOException {
+            LOG.info("region closed: " + regionInfo.getRegionNameAsString());
 
-          // Mark the Region as unavailable in the appropriate meta table
-
-          if (!metaRegionAvailable()) {
             // We can't proceed unless the meta region we are going to update
-            // is online. metaRegionAvailable() has put this operation on the
+            // is online. metaRegionAvailable() will put this operation on the
             // delayedToDoQueue, so return true so the operation is not put 
             // back on the toDoQueue
+
+            if (metaRegionAvailable()) {
+              // offline the region in meta and then note that we've offlined
+              // the region. 
+              HRegion.offlineRegionInMETA(server, metaRegionName,
+                  regionInfo);
+              master.regionManager.regionOfflined(regionInfo.getRegionName());
+            }
             return true;
           }
+        }.doWithRetries();
+        result = result == null ? true : result;
 
-          if (offlineRegion) {
-            // offline the region in meta and then note that we've offlined the
-            // region. 
-            HRegion.offlineRegionInMETA(server, metaRegionName,
-                regionInfo);
-            master.regionManager.regionOfflined(regionInfo.getRegionName());
-          }
-          return true;
-        }
-    }.doWithRetries();
+    } else if (reassignRegion) {
+      // we are reassigning the region eventually, so set it unassigned
+      master.regionManager.setUnassigned(regionInfo);
+    }
 
     return result == null ? true : result;
   }

src/java/org/apache/hadoop/hbase/master/ServerManager.java

@@ -499,19 +499,17 @@ class ServerManager implements HConstants {
       // the ProcessRegionClose going on asynchronously.
       master.regionManager.noLongerUnassigned(region);
 
-      if (!reassignRegion) {
-        // either the region is being offlined or deleted. we want to do those 
-        // operations asynchronously, so we'll creating a todo item for that.
-        try {
-          master.toDoQueue.put(new ProcessRegionClose(master, region, 
-              offlineRegion));
-        } catch (InterruptedException e) {
-          throw new RuntimeException(
-              "Putting into toDoQueue was interrupted.", e);
-        }
-      } else {
-        // we are reassigning the region eventually, so set it unassigned
-        master.regionManager.setUnassigned(region);
+      // NOTE: we cannot put the region into unassignedRegions as that
+      //       changes the ordering of the messages we've received. In
+      //       this case, a close could be processed before an open
+      //       resulting in the master not agreeing on the region's
+      //       state.
+      try {
+        master.toDoQueue.put(new ProcessRegionClose(master, region, 
+            offlineRegion, reassignRegion));
+      } catch (InterruptedException e) {
+        throw new RuntimeException(
+            "Putting into toDoQueue was interrupted.", e);
       }
     }
   }