HBASE-9092 OpenRegion could be ignored by mistake

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

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

@@ -71,9 +71,7 @@ import org.apache.hadoop.hbase.master.handler.EnableTableHandler;
 import org.apache.hadoop.hbase.master.handler.MergedRegionHandler;
 import org.apache.hadoop.hbase.master.handler.OpenedRegionHandler;
 import org.apache.hadoop.hbase.master.handler.SplitRegionHandler;
-import org.apache.hadoop.hbase.regionserver.RegionAlreadyInTransitionException;
 import org.apache.hadoop.hbase.regionserver.RegionOpeningState;
-import org.apache.hadoop.hbase.regionserver.RegionServerStoppedException;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.KeyLocker;
 import org.apache.hadoop.hbase.util.Pair;
@@ -923,11 +921,6 @@ public class AssignmentManager extends ZooKeeperListener {
               + " or not on the expected server");
             return;
           }
-          // Handle this the same as if it were opened and then closed.
-          regionState = regionStates.updateRegionState(rt, RegionState.State.CLOSED);
-          // 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)
-          if (regionState != null) {
           AtomicInteger failedOpenCount = failedOpenTracker.get(encodedName);
           if (failedOpenCount == null) {
             failedOpenCount = new AtomicInteger();
@@ -937,12 +930,16 @@ public class AssignmentManager extends ZooKeeperListener {
             failedOpenTracker.put(encodedName, failedOpenCount);
           }
           if (failedOpenCount.incrementAndGet() >= maximumAttempts) {
-              regionStates.updateRegionState(
+            regionStates.updateRegionState(rt, RegionState.State.FAILED_OPEN);
-                regionState.getRegion(), RegionState.State.FAILED_OPEN);
             // remove the tracking info to save memory, also reset
             // the count for next open initiative
             failedOpenTracker.remove(encodedName);
           } else {
+            // Handle this the same as if it were opened and then closed.
+            regionState = regionStates.updateRegionState(rt, RegionState.State.CLOSED);
+            if (regionState != null) {
+              // 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.getRegion(), sn, true);
               this.executorService.submit(new ClosedRegionHandler(server,
                 this, regionState.getRegion()));
@@ -1757,10 +1754,10 @@ public class AssignmentManager extends ZooKeeperListener {
       case CLOSING:
       case PENDING_CLOSE:
       case FAILED_CLOSE:
+      case FAILED_OPEN:
         unassign(region, state, -1, null, false, null);
         state = regionStates.getRegionState(region);
         if (state.isOffline()) break;
-      case FAILED_OPEN:
       case CLOSED:
         LOG.debug("Forcing OFFLINE; was=" + state);
         state = regionStates.updateRegionState(

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

@@ -248,7 +248,7 @@ public class RegionStates {
 
     if (state == State.FAILED_CLOSE || state == State.FAILED_OPEN) {
       LOG.warn("Failed to transition " + hri.getShortNameToLog()
-        + " on " + serverName + ": " + state);
+        + " on " + serverName + ", set to " + state);
     }
 
     String regionName = hri.getEncodedName();

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

@@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.master;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -392,6 +394,8 @@ public class TestAssignmentManagerOnCluster {
 
       RegionState state = am.getRegionStates().getRegionState(hri);
       assertEquals(RegionState.State.FAILED_OPEN, state.getState());
+      // Failed to open since no plan, so it's on no server
+      assertNull(state.getServerName());
 
       MockLoadBalancer.controledRegion = null;
       master.assignRegion(hri);
@@ -436,6 +440,10 @@ public class TestAssignmentManagerOnCluster {
 
       RegionState state = am.getRegionStates().getRegionState(hri);
       assertEquals(RegionState.State.FAILED_OPEN, state.getState());
+      // Failed to open due to file system issue. Region state should
+      // carry the opening region server so that we can force close it
+      // later on before opening it again. See HBASE-9092.
+      assertNotNull(state.getServerName());
 
       // remove the blocking file, so that region can be opened
       fs.delete(regionDir, true);