Apache9 commented on code in PR #6797:
URL: https://github.com/apache/hbase/pull/6797#discussion_r2103960853
##########
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:
Oh, in the if condition you call setState, I misread the code, I thought it
is isInState.
Then the logic is fine. But better to add more comments to say why here we
do not need to treat CLOSED state specially.
And I suggest that we use two ProcedureFutureUtil.suspendIfNecessary calls
for these two conditions, so we do not need to add extra check in
closeRegionAfterUpdatingMeta, since for the region in FAILED_OPEN state, the
only action after updating meta is to change the state.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
