junegunn commented on code in PR #6797:
URL: https://github.com/apache/hbase/pull/6797#discussion_r2105731225


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java:
##########
@@ -392,10 +399,26 @@ private void closeRegion(MasterProcedureEnv env, 
RegionStateNode regionNode)
     ) {
       return;
     }
-    if (regionNode.isInState(State.OPEN, State.CLOSING, State.MERGING, 
State.SPLITTING)) {
-      // this is the normal case
-      ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture,
-        env.getAssignmentManager().regionClosing(regionNode), env,
+
+    CompletableFuture<Void> future = null;
+    if (regionNode.isInState(STATES_EXPECTED_ON_CLOSING)) {
+      // This is the normal case
+      future = env.getAssignmentManager().regionClosing(regionNode);
+    } else if (regionNode.setState(State.CLOSED, State.FAILED_OPEN)) {
+      // FAILED_OPEN doesn't need further transition, immediately mark the 
region as closed
+      AssignmentManager am = env.getAssignmentManager();
+      am.getRegionStates().removeFromFailedOpen(regionNode.getRegionInfo());
+      future = am.getRegionStateStore().updateRegionLocation(regionNode);

Review Comment:
   
   > And I suggest that we use two ProcedureFutureUtil.suspendIfNecessary calls 
for these two conditions,
   
   Addressed that in 718156ed93907b4f99ab9c8d55e58afff89ed121. I also added 
more comments.
   
   > so we do not need to add extra check in closeRegionAfterUpdatingMeta
   
   The extra check is still there for the reason mentioned above.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to