mnpoonia opened a new pull request, #8622:
URL: https://github.com/apache/hbase/pull/8622
## What changes were proposed in this pull request?
`OpenRegionProcedure#restoreSucceedState()` is called on master-failover
restore, once
per region, via `RegionRemoteProcedureBase#stateLoaded()`. It
unconditionally forced the
region into `OPEN`, regardless of the actual persisted `transitionCode`,
which can be
`FAILED_OPEN`. The method's signature only received `seqId`, not
`transitionCode`, so it
was structurally unable to branch on the real outcome.
Concretely: an RS reports `FAILED_OPEN`; the master persists
`state=REPORT_SUCCEED, transitionCode=FAILED_OPEN` on the
`OpenRegionProcedure`, but the
in-memory `RegionState.State` is left untouched (still `OPENING`, since
`AssignmentManager#regionFailedOpen(regionNode, false)` on the live path
does not update
`RegionState.State`). If the master crashes/restarts before this reaches
`confirmOpened()`
and before `hbase:meta` is updated, `restoreSucceedState()` sees the region
isn't `OPEN`
yet and force-transitions it to `OPEN` anyway, then persists that (false)
`OPEN` state to
`hbase:meta`. There is no rollback anywhere in this procedure chain (by
design -
forward-only), so nothing downstream can detect or correct it; the region
silently looks
healthy in meta while no RegionServer is actually serving it.
By contrast, `CloseRegionProcedure#restoreSucceedState()` is safe doing the
equivalent
unconditional force, because CLOSE has no failure-variant transition code at
the master
side (`checkTransition`/`updateTransitionWithoutPersistingToMeta` both assert
`transitionCode == CLOSED`).
This is not a regression - the logic is unchanged (modulo spotless
formatting) since it
was introduced in HBASE-22365 (2019-05-10).
### The fix
- Widen `RegionRemoteProcedureBase#restoreSucceedState()` to also receive
the persisted
`transitionCode`, passed through from `stateLoaded()`.
- `OpenRegionProcedure#restoreSucceedState()` now branches: `FAILED_OPEN`
calls
`AssignmentManager#regionFailedOpen(regionNode, false)`, mirroring exactly
what the live
`reportTransition`/`updateTransitionWithoutPersistingToMeta` path already
does for the
same transition code; the existing `OPENED` handling is unchanged.
- `CloseRegionProcedure#restoreSucceedState()` accepts the new parameter and
ignores it,
since CLOSE has no failure variant.
After the fix, a restore-time `FAILED_OPEN` puts the region back into the
same
retryable path (`OPENING`, cleared location) that
`TransitRegionStateProcedure#confirmOpened()`
already drives on the live path - the region gets reassigned/retried through
the normal
flow instead of being falsely marked `OPEN`.
## Why are the changes needed?
To prevent a region from being silently, durably marked `OPEN` in
`hbase:meta` after a
master restart, when in reality no RegionServer opened it. Since HBase has
no rollback
mechanism for these forward-only assignment procedures, this bug is otherwise
unrecoverable except by manual detection and intervention.
## Does this PR introduce any user-facing change?
No.
## Is there a corresponding Apache JIRA?
Yes: [HBASE-30357](https://issues.apache.org/jira/browse/HBASE-30357)
## How was this patch tested?
Added `TestOpenRegionProcedureRestoreFailedOpen`, extending
`TestAssignmentManagerBase`. A
custom mock RS executor reports `FAILED_OPEN` on the first open attempt,
then - while
holding the `RegionStateNode` lock, simulating the point right after a crash
where the
child procedure has not yet resumed its own `execute()` - directly invokes
the
`TransitRegionStateProcedure#stateLoaded()` hook that a real master restart
would trigger,
and records the region's state immediately after. Confirmed the test fails
against
unmodified code with the exact predicted `OPEN` state, and passes after the
fix. Also ran
`TestAssignmentManager`, `TestTransitRegionStateProcedure`,
`TestOpenRegionProcedureHang`,
`TestOpenRegionProcedureBackoff`, `TestRollbackSCP`, and
`TestSCPGetRegionsRace` with no
regressions.
--
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]
