Fix #1025: self-fenced primary deadlocks forever on an unreachable goal state#1158
Merged
Merged
Conversation
check_for_network_partitions() in service_keeper.c self-fences a node into demote_timeout based purely on its own local current_role, independent of whatever goalState the monitor last assigned. If that goalState (e.g. wait_primary, assigned for an unrelated benign reason like a peer entering maintenance) has no FSM edge reachable from demote_timeout, the keeper fatals forever with "does not know how to reach state X from demote_timeout", and the rest of the formation stays stuck waiting on it. Add a guard in ProceedGroupStateFromContext: when a node reports demote_timeout but its assigned goal isn't one demote_timeout can actually reach, re-target it to demoted -- a real demote_timeout -> demoted FSM edge, and the safe, conservative choice. That guard exposed a second, pre-existing gap in GetPrimaryOrDemotedNodeInGroupFromList(): its fallback loop only recognized the transitional states leading up to a demotion (draining, demote_timeout, prepare_maintenance) via StateBelongsToPrimary(), never the terminal demoted state itself, unlike IsDemotedPrimary() right below it which already had to special-case this. Previously unreachable because every existing failover path already has a new primary by the time the old one reports fully demoted; this fix's self-fence recovery path is the first place a node can reach demoted with no promotion ever having happened, which surfaced the gap as a monitor-side ERROR crash-looping the node's node_active() calls. demoted is a safe landing spot but not a self-healing one: with no failover ever triggered, the peer never becomes a promotion candidate, so there's no existing mechanism (perform_failover included) to elect a new primary from here. Recovering the formation to a working primary again is a separate, pre-existing gap and out of scope for this fix.
…ad end Two more questions worth capturing alongside the #1025 deadlock fix: - What happens to the former secondary while the primary is disconnected and self-fencing, and afterwards? Nothing: entering maintenance is fully independent of the primary, and the MAINTENANCE early-exit in ProceedGroupStateFromContext means the monitor won't touch it again until an explicit stop_maintenance() call, regardless of what the primary does in the meantime. test_002 asserts this directly. - How do we repair the cluster and converge back to a working primary? With today's tools, we can't. test_003 demonstrates the dead end: stop_maintenance() assigns catchingup (not report_lsn), since IsFailoverInProgress() is false at that point -- so the secondary retries a replication connection to the demoted, stopped primary forever. perform_failover() and perform_promotion() both refuse too, since neither a demoted primary nor a catchingup secondary satisfies their preconditions. Recovering formations stuck in this shape is a separate, pre-existing gap from #1025 and remains unsolved.
stop_maintenance() found the demoted primary via GetPrimaryOrDemotedNodeInGroupFromList() (now correctly recognizing it, per the previous commit's fix) but still assigned the rejoining node "catchingup" -- meaning "stream from the primary". A demoted primary's Postgres is stopped, so that retried a doomed replication connection forever, with no automatic or documented manual path back (verified: perform_failover() and perform_promotion() both refuse too, since neither a demoted primary nor a catchingup secondary satisfies their preconditions). When the found primary is IsDemotedPrimary() -- fully converged to demoted, nothing left running to stream from -- assign report_lsn instead. That's what seeds the recovery: once the node reports its LSN, ProceedGroupStateForMSFailover()'s candidate scan (already existing code) picks up the demoted primary too, both report their LSN, and the normal election promotes the most advanced one. The demoted -> report_lsn FSM edge (fsm.c:854, fsm_report_lsn_and_drop_replication_slots) was already built for exactly this: "used when a former primary node has been demoted and gets back online during the secondary election" -- it just had no way to get seeded in a 2-node group where the only other node was parked in maintenance the whole time. Extended demote_timeout_wait_primary_deadlock.pgaf: test_002 confirms the secondary is unaffected by the primary's self-fence (and pins down exactly which monitor-side rule releases it from wait_maintenance), test_003 confirms disable maintenance now converges the formation back to a working primary/secondary pair end to end.
Comments had grown to the point of drowning out the actual scenario.
Cut the per-line source citations and restated rationale down to what's
needed to follow the steps; the commit messages carry the full
investigation. Also label the two bare `sql monitor {}` blocks (no
`expect`) as DEBUG -- they're just state snapshots printed for a human
reading the test output, not assertions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1025.
Bug
check_for_network_partitions()inservice_keeper.cself-fences a node intodemote_timeoutbased purely on its own localcurrent_role, independent of whatevergoalStatethe monitor last assigned. If that goal state (e.g.wait_primary, assigned for an unrelated benign reason such as a peer entering maintenance) has no FSM edge reachable fromdemote_timeout, the keeper fatals forever:Fix
group_state_machine.c: a guard inProceedGroupStateFromContextre-targets a node reportingdemote_timeoutwith an unreachable goal todemoted— a realdemote_timeout -> demotedFSM edge, and the safe, conservative choice.node_metadata.c:GetPrimaryOrDemotedNodeInGroupFromList()didn't recognize a node that had fully converged todemoted(only the transitional states leading up to a demotion), a prerequisite gap the guard above exposed. Extended it to matchIsDemotedPrimary()'s existing idiom.node_active_protocol.c:stop_maintenance()assignedcatchingup(stream from the current primary) even when the only "primary" found is fullydemoted— Postgres stopped, nothing to stream from — so a node coming out of maintenance retried a doomed connection forever with no way back. It now assignsreport_lsnin that case, which seeds the existing candidate-election machinery and lets the formation actually recover a working primary.Test
tests/tap/specs/demote_timeout_wait_primary_deadlock.pgafreproduces the deadlock end to end and verifies the fix in three steps: the self-fence recovers todemotedinstead of crash-looping, the secondary is confirmed unaffected while the primary is down, and disabling maintenance afterwards converges the formation back to a working primary/secondary pair.