You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "lokiore (via GitHub)" <gi...@apache.org> on 2026/10/20 20:36:22 UTC
[PR] PHOENIX-8027 :- Allow consistent failover to be initiated only from ACTIVE_IN_SYNC [phoenix]
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-8027 :- Allow consistent failover to be initiated only from ACTIVE_IN_SYNC [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2640:
URL: https://github.com/apache/phoenix/pull/2640#discussion_r4333403517
##########
phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java:
##########
@@ -743,6 +747,23 @@ public void testE2EFailoverFromActiveNotInSyncWithAutomaticStateTransitions() th
waitForHAGroupState(cluster2HAManager, haGroupName,
HAGroupStoreRecord.HAGroupState.DEGRADED_STANDBY);
+ try {
+ cluster1HAManager.initiateFailoverOnActiveCluster(haGroupName);
+ fail("Failover from ACTIVE_NOT_IN_SYNC should be rejected");
+ } catch (InvalidClusterRoleTransitionException e) {
+ assertTrue(e.getMessage(), e.getMessage().contains("store-and-forward"));
+ }
+ // Rejection lands well inside the anti-flap window, so the group cannot have resynced yet.
Review Comment:
The assertion that cluster1 is still ACTIVE_NOT_IN_SYNC right after the rejection depends on timing. It holds only if the check runs before the anti-flap wait ends and the forwarder moves the group back to
ACTIVE_IN_SYNC. On a slow or loaded CI run, the group can already have resynced by then, and the test fails even though the rejection behaved correctly.
What we care about here is that the rejected failover changed nothing. So it might be more robust to assert that neither cluster entered a failover state, for example:
HAGroupStoreRecord.HAGroupState c1 =
cluster1HAManager.getHAGroupStoreRecord(haGroupName).get().getHAGroupState();
assertTrue("Rejected failover must not move the active: " + c1,
c1 == HAGroupStoreRecord.HAGroupState.ACTIVE_NOT_IN_SYNC
|| c1 == HAGroupStoreRecord.HAGroupState.ACTIVE_IN_SYNC);
HAGroupStoreRecord.HAGroupState c2 =
cluster2HAManager.getHAGroupStoreRecord(haGroupName).get().getHAGroupState();
assertTrue("Rejected failover must not move the standby: " + c2,
c2 == HAGroupStoreRecord.HAGroupState.DEGRADED_STANDBY
|| c2 == HAGroupStoreRecord.HAGroupState.STANDBY);
That still catches a failover that slipped through, which would show as ACTIVE_IN_SYNC_TO_STANDBY / ACTIVE_NOT_IN_SYNC_TO_STANDBY on cluster1 or STANDBY_TO_ACTIVE on cluster2, without depending on the
anti-flap timing.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-8027 :- Allow consistent failover to be initiated only from ACTIVE_IN_SYNC [phoenix]
Posted by "lokiore (via GitHub)" <gi...@apache.org>.
lokiore commented on code in PR #2640:
URL: https://github.com/apache/phoenix/pull/2640#discussion_r4340235143
##########
phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java:
##########
@@ -743,6 +747,23 @@ public void testE2EFailoverFromActiveNotInSyncWithAutomaticStateTransitions() th
waitForHAGroupState(cluster2HAManager, haGroupName,
HAGroupStoreRecord.HAGroupState.DEGRADED_STANDBY);
+ try {
+ cluster1HAManager.initiateFailoverOnActiveCluster(haGroupName);
+ fail("Failover from ACTIVE_NOT_IN_SYNC should be rejected");
+ } catch (InvalidClusterRoleTransitionException e) {
+ assertTrue(e.getMessage(), e.getMessage().contains("store-and-forward"));
+ }
+ // Rejection lands well inside the anti-flap window, so the group cannot have resynced yet.
Review Comment:
Good catch, thanks. Instead of widening the assertion, I moved the `ReplicationLogGroup` creation (which owns the forwarder) to after the rejection checks. Before that point nothing in the test can move the group from ACTIVE_NOT_IN_SYNC back to ACTIVE_IN_SYNC: HA-store prewarm is off in this IT and the test issues no mutations. So the strict ACTIVE_NOT_IN_SYNC / DEGRADED_STANDBY assertions no longer depend on the anti-flap timing. The group then starts in store-and-forward, the forwarder finds no backlog, and the rest of the test (back to ACTIVE_IN_SYNC, then failover succeeds) is unchanged. Fixed in 432e581c71.
_(Posted by Loki's Claude companion)_
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-8027 :- Allow consistent failover to be initiated only from ACTIVE_IN_SYNC [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana merged PR #2640:
URL: https://github.com/apache/phoenix/pull/2640
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org