HBASE-19127: Set State.SPLITTING, MERGING, MERGING_NEW, SPLITTING_NEW properly in RegionStatesNode

Signed-off-by: Michael Stack <stack@apache.org>

hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto

@@ -256,16 +256,14 @@ message SplitTableRegionStateData {
 enum MergeTableRegionsState {
   MERGE_TABLE_REGIONS_PREPARE = 1;
   MERGE_TABLE_REGIONS_PRE_OPERATION = 2;
-  MERGE_TABLE_REGIONS_MOVE_REGION_TO_SAME_RS = 3;
+  MERGE_TABLE_REGIONS_PRE_MERGE_OPERATION = 3;
-  MERGE_TABLE_REGIONS_PRE_MERGE_OPERATION = 4;
+  MERGE_TABLE_REGIONS_CLOSE_REGIONS = 4;
-  MERGE_TABLE_REGIONS_SET_MERGING_TABLE_STATE = 5;
+  MERGE_TABLE_REGIONS_CREATE_MERGED_REGION = 5;
-  MERGE_TABLE_REGIONS_CLOSE_REGIONS = 6;
+  MERGE_TABLE_REGIONS_PRE_MERGE_COMMIT_OPERATION = 6;
-  MERGE_TABLE_REGIONS_CREATE_MERGED_REGION = 7;
+  MERGE_TABLE_REGIONS_UPDATE_META = 7;
-  MERGE_TABLE_REGIONS_PRE_MERGE_COMMIT_OPERATION = 8;
+  MERGE_TABLE_REGIONS_POST_MERGE_COMMIT_OPERATION = 8;
-  MERGE_TABLE_REGIONS_UPDATE_META = 9;
+  MERGE_TABLE_REGIONS_OPEN_MERGED_REGION = 9;
-  MERGE_TABLE_REGIONS_POST_MERGE_COMMIT_OPERATION = 10;
+  MERGE_TABLE_REGIONS_POST_OPERATION = 10;
-  MERGE_TABLE_REGIONS_OPEN_MERGED_REGION = 11;
-  MERGE_TABLE_REGIONS_POST_OPERATION = 12;
 }
 
 message MergeTableRegionsStateData {

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

@@ -1532,6 +1532,11 @@ public class AssignmentManager implements ServerListener {
     // regionStates#getRegionsOfTable
     final RegionStateNode node = regionStates.getOrCreateRegionNode(parent);
     node.setState(State.SPLIT);
+    final RegionStateNode nodeA = regionStates.getOrCreateRegionNode(daughterA);
+    nodeA.setState(State.SPLITTING_NEW);
+    final RegionStateNode nodeB = regionStates.getOrCreateRegionNode(daughterB);
+    nodeB.setState(State.SPLITTING_NEW);
+
     regionStateStore.splitRegion(parent, daughterA, daughterB, serverName);
     if (shouldAssignFavoredNodes(parent)) {
       List<ServerName> onlineServers = this.master.getServerManager().getOnlineServersList();

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

@@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.master.CatalogJanitor;
 import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.master.MasterFileSystem;
 import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.RegionState.State;
 import org.apache.hadoop.hbase.master.normalizer.NormalizationPlan;
 import org.apache.hadoop.hbase.master.procedure.AbstractStateMachineTableProcedure;
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
@@ -222,10 +223,6 @@ public class MergeTableRegionsProcedure
         break;
       case MERGE_TABLE_REGIONS_PRE_MERGE_OPERATION:
         preMergeRegions(env);
-        setNextState(MergeTableRegionsState.MERGE_TABLE_REGIONS_SET_MERGING_TABLE_STATE);
-        break;
-      case MERGE_TABLE_REGIONS_SET_MERGING_TABLE_STATE:
-        setRegionStateToMerging(env);
         setNextState(MergeTableRegionsState.MERGE_TABLE_REGIONS_CLOSE_REGIONS);
         break;
       case MERGE_TABLE_REGIONS_CLOSE_REGIONS:
@@ -295,14 +292,9 @@ public class MergeTableRegionsProcedure
       case MERGE_TABLE_REGIONS_CLOSE_REGIONS:
         rollbackCloseRegionsForMerge(env);
         break;
-      case MERGE_TABLE_REGIONS_SET_MERGING_TABLE_STATE:
-        setRegionStateToRevertMerging(env);
-        break;
       case MERGE_TABLE_REGIONS_PRE_MERGE_OPERATION:
         postRollBackMergeRegions(env);
         break;
-      case MERGE_TABLE_REGIONS_MOVE_REGION_TO_SAME_RS:
-        break; // nothing to rollback
       case MERGE_TABLE_REGIONS_PREPARE:
         break;
       default:
@@ -513,6 +505,8 @@ public class MergeTableRegionsProcedure
       return false;
     }
 
+    // Update region states to Merging
+    setRegionStateToMerging(env);
     return true;
   }
 
@@ -560,16 +554,23 @@ public class MergeTableRegionsProcedure
    * @throws IOException
    */
   public void setRegionStateToMerging(final MasterProcedureEnv env) throws IOException {
-    //transition.setTransitionCode(TransitionCode.READY_TO_MERGE);
+    // Set State.MERGING to regions to be merged
+    RegionStates regionStates = env.getAssignmentManager().getRegionStates();
+    regionStates.getRegionNode(regionsToMerge[0]).setState(State.MERGING);
+    regionStates.getRegionNode(regionsToMerge[1]).setState(State.MERGING);
   }
 
   /**
    * Rollback the region state change
+   * Not used for now, since rollbackCloseRegionsForMerge() will mark regions as OPEN
    * @param env MasterProcedureEnv
    * @throws IOException
    */
-  private void setRegionStateToRevertMerging(final MasterProcedureEnv env) throws IOException {
+  private void setRegionStateBackToOpen(final MasterProcedureEnv env) throws IOException {
-    //transition.setTransitionCode(TransitionCode.MERGE_REVERTED);
+    // revert region state to Open
+    RegionStates regionStates = env.getAssignmentManager().getRegionStates();
+    regionStates.getRegionNode(regionsToMerge[0]).setState(State.OPEN);
+    regionStates.getRegionNode(regionsToMerge[1]).setState(State.OPEN);
   }
 
   /**
@@ -591,6 +592,10 @@ public class MergeTableRegionsProcedure
     mergeStoreFiles(env, regionFs2, regionFs.getMergesDir());
 
     regionFs.commitMergedRegion(mergedRegion);
+
+    //Prepare to create merged regions
+    env.getAssignmentManager().getRegionStates().
+        getOrCreateRegionNode(mergedRegion).setState(State.MERGING_NEW);
   }
 
   /**

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

@@ -67,7 +67,7 @@ public class RegionStates {
   };
 
   protected static final State[] STATES_EXPECTED_ON_CLOSE = new State[] {
-    State.SPLITTING, State.SPLIT, // ServerCrashProcedure
+    State.SPLITTING, State.SPLIT, State.MERGING, // ServerCrashProcedure
     State.OPEN,                   // enabled/open
     State.CLOSING                 // already in-progress (retrying)
   };

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

@@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.UnknownRegionException;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
 import org.apache.hadoop.hbase.client.MasterSwitchType;
 import org.apache.hadoop.hbase.client.Mutation;
@@ -406,31 +407,32 @@ public class SplitTableRegionProcedure
   public boolean prepareSplitRegion(final MasterProcedureEnv env) throws IOException {
     // Check whether the region is splittable
     RegionStateNode node =
-      env.getAssignmentManager().getRegionStates().getRegionNode(getParentRegion());
+        env.getAssignmentManager().getRegionStates().getRegionNode(getParentRegion());
-    RegionInfo parentHRI = null;
-    if (node != null) {
-      parentHRI = node.getRegionInfo();
 
-      // Lookup the parent HRI state from the AM, which has the latest updated info.
+    if (node == null) {
-      // Protect against the case where concurrent SPLIT requests came in and succeeded
+      throw new UnknownRegionException(getParentRegion().getRegionNameAsString());
-      // just before us.
+    }
-      if (node.isInState(State.SPLIT)) {
-        LOG.info("Split of " + parentHRI + " skipped; state is already SPLIT");
-        return false;
-      }
-      if (parentHRI.isSplit() || parentHRI.isOffline()) {
-        LOG.info("Split of " + parentHRI + " skipped because offline/split.");
-        return false;
-      }
 
-      // expected parent to be online or closed
+    RegionInfo parentHRI = node.getRegionInfo();
-      if (!node.isInState(EXPECTED_SPLIT_STATES)) {
+    // Lookup the parent HRI state from the AM, which has the latest updated info.
-        // We may have SPLIT already?
+    // Protect against the case where concurrent SPLIT requests came in and succeeded
-        setFailure(new IOException("Split " + parentHRI.getRegionNameAsString() +
+    // just before us.
-            " FAILED because state=" + node.getState() + "; expected " +
+    if (node.isInState(State.SPLIT)) {
-            Arrays.toString(EXPECTED_SPLIT_STATES)));
+      LOG.info("Split of " + parentHRI + " skipped; state is already SPLIT");
-        return false;
+      return false;
-      }
+    }
+    if (parentHRI.isSplit() || parentHRI.isOffline()) {
+      LOG.info("Split of " + parentHRI + " skipped because offline/split.");
+      return false;
+    }
+
+    // expected parent to be online or closed
+    if (!node.isInState(EXPECTED_SPLIT_STATES)) {
+      // We may have SPLIT already?
+      setFailure(new IOException("Split " + parentHRI.getRegionNameAsString() +
+          " FAILED because state=" + node.getState() + "; expected " +
+          Arrays.toString(EXPECTED_SPLIT_STATES)));
+      return false;
     }
 
     // Since we have the lock and the master is coordinating the operation
@@ -442,6 +444,10 @@ public class SplitTableRegionProcedure
           " failed due to split switch off"));
       return false;
     }
+
+    // set node state as SPLITTING
+    node.setState(State.SPLITTING);
+
     return true;
   }