lokiore opened a new pull request, #2640:
URL: https://github.com/apache/phoenix/pull/2640
### What changes were proposed in this pull request?
This targets the **Consistent Failover feature branch**
(`PHOENIX-7562-feature-new`). Initiating failover is now allowed only from
`ACTIVE_IN_SYNC`.
- `HAGroupStoreManager.initiateFailoverOnActiveCluster` throws
`InvalidClusterRoleTransitionException` for every state except
`ACTIVE_IN_SYNC`. Previously `ACTIVE_NOT_IN_SYNC` was also accepted and moved
to `ACTIVE_NOT_IN_SYNC_TO_STANDBY`. For `ACTIVE_NOT_IN_SYNC` the message tells
the operator to wait for the store-and-forward backlog to drain and then retry.
- `PhoenixHAAdminTool initiate-failover` applies the same check and returns
`RET_VALIDATION_ERROR`. From `ACTIVE_NOT_IN_SYNC` it also prints guidance:
check the peer, wait for the group to return to `ACTIVE_IN_SYNC`, then retry.
The help text and Javadoc are updated.
- The `ACTIVE_NOT_IN_SYNC_TO_STANDBY` enum value, the `ANIS -> ANISTS` entry
in the transition table, and the `ANISTS -> ATS` mapping in
`setHAGroupStatusToSync` are **kept**. A group already in `ANISTS` (in flight
during an upgrade, or driven by an older client or tool) can still drain and
complete. Only new failovers from `ANIS` are rejected.
- A race where the group moves `AIS -> ANIS` between the state check and the
`ATS` write is still safe. `setHAGroupStatusIfNeeded` validates the transition
against the current record, so `ANIS -> ATS` is rejected. The admin tool maps
that late rejection to `RET_VALIDATION_ERROR` (with the store-and-forward
guidance if the group is now `ACTIVE_NOT_IN_SYNC`) instead of the generic
update error.
- TLA+ model (`src/main/tla/ConsistentFailover`): `AdminStartFailover` now
requires `AIS` and only produces `ATS`. `ANISTS` is no longer produced by any
action. It stays in `Types` and `ANISTSToATS` to mirror the implementation's
transition table, and comments and markdown note that the action is never
enabled.
### Why are the changes needed?
In `ACTIVE_NOT_IN_SYNC` the active is in store-and-forward: writes are
buffered in the local OUT directory and forwarded to the peer asynchronously.
Testing found that failing over in this state promotes a standby that may not
have received the forwarded backlog yet. The intended guarantee is zero RPO. To
keep it, failover must either block on the drain or fail fast. Failing fast is
simpler and easier for operators to reason about: the group returns to
`ACTIVE_IN_SYNC` automatically once the forwarder drains the backlog and the
anti-flapping wait passes, and failover can then be retried.
### Does this PR introduce _any_ user-facing change?
Yes, on this feature branch.
- `initiate-failover` (and
`HAGroupStoreManager.initiateFailoverOnActiveCluster`) from
`ACTIVE_NOT_IN_SYNC` now fails with `RET_VALIDATION_ERROR` /
`InvalidClusterRoleTransitionException` and leaves the state of both clusters
unchanged. Previously it moved the group to `ACTIVE_NOT_IN_SYNC_TO_STANDBY`.
- Failover from `ACTIVE_IN_SYNC` is unchanged.
- Only the code that runs `initiate-failover` enforces the rule (it runs in
the admin tool's JVM). Upgrade the admin tool along with the clusters; an older
tool can still move a group from `ACTIVE_NOT_IN_SYNC` to
`ACTIVE_NOT_IN_SYNC_TO_STANDBY`, and that legacy path still drains and
completes.
New admin-tool output from `ACTIVE_NOT_IN_SYNC`:
```
✗ Cannot initiate failover from current state: ACTIVE_NOT_IN_SYNC
Cluster must be in ACTIVE_IN_SYNC state.
The cluster is in store-and-forward mode; the peer may not have all writes
yet.
1. Check the peer is reachable and healthy (run on each cluster):
get-cluster-role-record -g <group>
2. Wait for the backlog to drain; the group returns to ACTIVE_IN_SYNC
automatically.
3. Retry: initiate-failover -g <group>
```
### How was this patch tested?
-
`HAGroupStoreManagerIT#testE2EFailoverRejectedFromActiveNotInSyncUntilBackInSync`
replaces the ANIS failover E2E test. It moves the group to
`ANIS`/`DEGRADED_STANDBY` and asserts that failover is rejected with both
cluster states unchanged. It then waits for the forwarder to return the group
to `AIS`/`STANDBY`, and asserts that failover then completes (`ATS` ->
`STANDBY` / `STA` -> `AIS`) with the expected cluster roles.
- New `PhoenixHAAdminToolIT#testInitiateFailoverRejectedFromActiveNotInSync`
asserts `RET_VALIDATION_ERROR`, the guidance output, that no "Failover
initiated" line is printed, and that both cluster states are unchanged.
- Full `PhoenixHAAdminToolIT` (20), `HAGroupStoreManagerIT` (19),
`HAGroupStoreClientIT` (58), `HAGroupStoreRecordTest` and
`PhoenixHAAdminToolTest` (22) pass locally.
- TLA+: SANY passes. An exhaustive TLC run of `ConsistentFailover.cfg` (2
clusters × 2 RS) passes all 6 invariants and 9 action constraints. It explored
46,422,720 distinct states at depth 55 in 8 min 42 s, down from 70,543,872
because the ANIS failover branch is gone; README "Latest Results" is updated.
The liveness simulations were not rerun: admin actions carry no fairness and
the new next-state relation is a subset of the old one, so the fair behaviors
of the new spec are a subset of the old ones and the earlier `AbortCompletion`,
`DegradationRecovery`, and `FailoverCompletion` simulation results still apply.
### Follow-ups (not in this PR)
- Testing and review also found that `initiateFailoverOnActiveCluster` does
not check that the peer is in `STANDBY`/`DEGRADED_STANDBY` before writing
`ACTIVE_IN_SYNC_TO_STANDBY`, which the TLA+ model assumes. Re-running failover
against the new active during the brief `(ATS, AIS)` window after a failover
could leave both clusters in `ATS`. The model comments now state this as an
operator-side assumption; adding the peer check is a separate JIRA.
- Optionally model the legacy `ANIS -> ANISTS` path behind a constant, so
TLC keeps checking it while mixed versions are supported.
- Existing divergence on the legacy path: Java maps
`ACTIVE_NOT_IN_SYNC_TO_STANDBY` to the `ACTIVE` role (mutations not blocked),
while the TLA+ model treats `ANISTS` as `ACTIVE_TO_STANDBY` (blocked). This is
unchanged here, and the state is still reachable from older admin tools.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
