HBASE-30353 Prevent split parent regions from being re-opened - #8700
tanyastickles wants to merge 7 commits into
Conversation
…aster failover Split parents are permanently retired regions and must never be re-assigned. Two-part fix: 1. RegionStateStore.splitRegion() now writes SPLIT into info:state for the parent so that after a master failover, loadMeta reconstructs the parent with state=SPLIT rather than falling back to OFFLINE (the null-state path), which would make it eligible for re-assignment via processOfflineRegions(). 2. AssignmentManager guards isSplit() in both preTransitCheck (normal assign path) and createAssignProcedure (HBCK2/override/force path). The guard in createAssignProcedure sits before the override branch so that even force=true cannot bypass it. Together these cover the live-state case (state=SPLIT) and the post-failover case where an older meta row carries no info:state column and regionInfo is reconstructed with state=CLOSED but isSplit()=true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| // Persist the terminal SPLIT state so that after a master failover loadMeta reconstructs the | ||
| // parent as state=SPLIT rather than falling back to OFFLINE, which would make it eligible for | ||
| // re-assignment via processOfflineRegions. See HBASE-30353. | ||
| MetaTableAccessor.addRegionStateToPut(putParent, RegionInfo.DEFAULT_REPLICA_ID, |
There was a problem hiding this comment.
this is the main thing i want feedback on. how do we feel about persisting state=SPLIT to the metafile in addition to the split=true flag?
There was a problem hiding this comment.
correct me if i'm wrong, but it looks pretty common to have drift between in-memory state and state that's written to the metafile.
SPLITTING, SPLITTING_NEW, MERGING, FAILED_OPEN, and some others are all in memory state that don't get written to the meta. those are more transient than SPLIT.
we already keep track of whether the region is split in split=true, so i don't have have a strong opinion on approach here.
There was a problem hiding this comment.
So if we add this here, we wouldn't need the extra "isSplit" checks you added on AssignmentManager?
There was a problem hiding this comment.
we'd still need the isSplit check in AssignmentManager.createAssignProcedure (line 803). if the method is called with override=true, it would skip the preTransitCheck that validates that the region is CLOSED or OFFLINE.
There was a problem hiding this comment.
but elsewhere, we wouldn't need to add the additional isSplit checks
| if (!regionNode.isInState(expectedStates)) { | ||
| throw new DoNotRetryRegionException(UNEXPECTED_STATE_REGION + regionNode); | ||
| } | ||
| // if we don't write the state as SPLIT to the meta, we would need this check. |
There was a problem hiding this comment.
comments are for context and discussion. i'll clean this up once we decide on an approach (persist SPLIT to state or just check the isSplit flag.)
There was a problem hiding this comment.
Ok, after reviewing the state machine diagram on the official doc, it seems the SPLIT state should be a terminal state where a split region ends, so having a split region in CLOSED state seems inconsistent with the diagram. Therefore, setting the SPLIT state would be more compliant with the documentation.
tanyastickles
changed the title
2 participants
Issue: https://issues.apache.org/jira/browse/HBASE-30353
We ran into an issue where a split parent region came back online.
What happened was:
state=CLOSED, split=true, offline=true. (The in-memory state of the previous HMaster would have hadstate=SPLIT)