Skip to content

PHOENIX-7920 Add replay-side handling for direct DEGRADED_STANDBY → STANDBY_TO_ACTIVE transition (ANISTS → AISTS)#2577

Open
Himanshu-g81 wants to merge 8 commits into
apache:PHOENIX-7562-feature-newfrom
Himanshu-g81:PHOENIX-7920-pr
Open

PHOENIX-7920 Add replay-side handling for direct DEGRADED_STANDBY → STANDBY_TO_ACTIVE transition (ANISTS → AISTS)#2577
Himanshu-g81 wants to merge 8 commits into
apache:PHOENIX-7562-feature-newfrom
Himanshu-g81:PHOENIX-7920-pr

Conversation

@Himanshu-g81

Copy link
Copy Markdown
Contributor

Himanshu Gwalani added 6 commits July 16, 2026 17:01
…NC when nothing to rewind

On a restart while already STANDBY_TO_ACTIVE (e.g. an RS bounce after a direct DEGRADED_STANDBY -> STANDBY_TO_ACTIVE), enter SYNCED_RECOVERY only when a rewind is actually pending (lastRoundInSync behind lastRoundProcessed). When the rounds are equal there is nothing to rewind, so initialize directly to SYNC and let the first replay() promote immediately, instead of waiting ~one round window for the SYNCED_RECOVERY -> SYNC CAS that only fires inside the round-processing loop. Arm failoverPending with a plain set and emit distinct restart-init log lines for the two sub-cases.

Add regression guard testReplay_RestartIntoStandbyToActive_NothingToReplay_PromotesOnFirstTick (promotes on the first replay tick) and update the existing STANDBY_TO_ACTIVE init tests for the SYNC outcome. In StoreAndForwardFailoverIT, gate transitionCluster2's peer-reaction settle-poll to the DEGRADED_STANDBY target (with named constants) and note that PHOENIX_REPLICATION_REPLAY_ENABLED also gates the compaction consistency-point guard.

Document degradedListener's authoritative (non-CAS) set and the init-time compareAndSet(NOT_INITIALIZED, ...) yield-to-listener semantics.
…OVERY), not unconditional set

A spurious LOCAL STANDBY event arriving while already SYNC (e.g. an aborted-failover round trip, or a cache-reconnect redelivery) would otherwise flip SYNC -> SYNCED_RECOVERY and needlessly re-process the frontier round. CAS from DEGRADED only makes recovery safe by construction, symmetric with triggerFailoverListner.

- Add guard IT: Phase 1 proves a spurious STANDBY-while-SYNC stays SYNC; Phase 2 proves a genuine DEGRADED_STANDBY -> STANDBY still reaches SYNCED_RECOVERY. Adds effectiveLocalStateIs helper.

- Tighten HealthyStandbyToActiveKeepsSync: use failoverPending as the synchronization barrier and re-assert both invariants after settling.

- Document that the LOCAL HA-state listeners run on the notification callback and must be non-blocking.
@Himanshu-g81 Himanshu-g81 changed the title Phoenix-7920 Add replay-side handling for direct DEGRADED_STANDBY → STANDBY_TO_ACTIVE transition (ANISTS → AISTS) PHOENIX-7920 Add replay-side handling for direct DEGRADED_STANDBY → STANDBY_TO_ACTIVE transition (ANISTS → AISTS) Jul 21, 2026
Brings in PHOENIX-7874 (regenerate index mutations on standby, apache#2566) and PHOENIX-7938 (round-align the replay consistency point, apache#2570).

Conflict: phoenix-core/.../replication/ReplicationLogGroupIT.java. Both sides deduplicated the findLogFiles / assertTablesEqualAcrossClusters test helpers: upstream extracted them into a new ReplicationLogGroupBaseIT base class; this branch had extracted them into CrossClusterReplicationTestUtil. Took upstream's version wholesale -- its base-class extraction supersedes the local dedup, and CrossClusterReplicationTestUtil remains for StoreAndForwardFailoverIT (a separate HABaseIT hierarchy). No PHOENIX-7920 logic lived in this file.

Production ReplicationLogDiscoveryReplay.java auto-merged cleanly: the C2 recoveryListener/triggerFailover CAS state-machine changes and PHOENIX-7938's getConsistencyPoint round-alignment are orthogonal and consistent (7938 relies on the same SYNC invariant C2 protects). Verified: test-compile BUILD SUCCESS across production + all test/IT sources.

@ritegarg ritegarg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved from HAGroupStore perspective

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Shared cross-cluster helpers for replication ITs (extracted from ReplicationLogGroupIT). */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to reduce code duplication by refactoring ReplicationLogGroupIT

// would otherwise flip SYNC -> SYNCED_RECOVERY and needlessly re-process the frontier
// round. Symmetric with triggerFailoverListner below; contrast degradedListener above,
// whose unconditional fail-closed set() is intentional.
replicationReplayState.compareAndSet(ReplicationReplayState.DEGRADED,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value of compareAndSet is ignored ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just want to ensure we don't do any un-desirable state change and avoid race conditions. Have logged it also (in one of the existing log line)

// lastRoundInSync before shouldTriggerFailover() (which gates on SYNC) can promote.
// compareAndSet, not set: a listener firing while already SYNC (healthy failover) or
// SYNCED_RECOVERY (rewind already pending) must not clobber a good state.
replicationReplayState.compareAndSet(ReplicationReplayState.DEGRADED,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value of compareAndSet is ignored

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// concurrent LOCAL transition may have already advanced the state off NOT_INITIALIZED. When
// that happens the CAS no-ops and we deliberately keep the listener's value -- a live
// transition is more recent (and thus more authoritative) than the record snapshot read above.
if (HAGroupStoreRecord.HAGroupState.DEGRADED_STANDBY.equals(haGroupState)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Init-window race can leave a direct failover silently stuck (correctness / silent failure — narrow window, self-heals on RS restart)
ReplicationLogDiscoveryReplay.java:266 interacting with triggerFailoverListner at :178-180

Listeners are subscribed in init() before super.init() runs initializeLastRoundProcessed(), so a LOCAL transition can fire mid-init. The comment at :260-264 covers the
interleaving where the listener wins, but not this one:

  1. getHAGroupRecord() reads (slightly stale) DEGRADED_STANDBY → picks the DEGRADED branch.
  2. Before CAS(NOT_INITIALIZED, DEGRADED) at :266, the record flips to STANDBY_TO_ACTIVE; triggerFailoverListner runs compareAndSet(DEGRADED, SYNCED_RECOVERY) while state
    is still NOT_INITIALIZED → no-op, then failoverPending.set(true).
  3. Init resumes: CAS(NOT_INITIALIZED, DEGRADED) succeeds → state DEGRADED.
  4. Trailing arm-check at :316 tests the local haGroupState (DEGRADED_STANDBY), so it doesn't reconcile.

Result: state=DEGRADED + failoverPending=true, but nothing will move DEGRADED → SYNCED_RECOVERY (the record is already STANDBY_TO_ACTIVE and won't re-fire; a direct
failover never revisits STANDBY). shouldTriggerFailover() gates on SYNC, so promotion never happens until the RS restarts (the STANDBY_TO_ACTIVE init branch heals it).
Low-probability, but it's exactly the "legitimate signal silently dropped" case the degradedListener comment warns against.

Suggested fix: after the init branches resolve, reconcile — if failoverPending.get() is true and resolved state is DEGRADED, promote to SYNCED_RECOVERY (or re-derive as
the STANDBY_TO_ACTIVE branch does) and log a WARN.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, have updated in latest commit.

…ion test, dedup findLogFiles

- ReplicationLogDiscoveryReplay: capture and log the compareAndSet result
  (rewindScheduled) in recoveryListener and triggerFailoverListner so the
  guarded-transition outcome is visible in logs.
- ReplicationLogDiscoveryReplayTestIT: add a deterministic init-window race
  regression test (direct DEGRADED_STANDBY -> STANDBY_TO_ACTIVE reconciles)
  and fix a stale ordering comment.
- Route findLogFiles through CrossClusterReplicationTestUtil and remove the
  pass-through delegate; repoint ReplicationLogGroupBaseIT and
  ReplicationLogGroupIT call sites to the shared util.
@Himanshu-g81
Himanshu-g81 requested a review from tkhurana July 23, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants