Monitor: detect and recover from fast_forward candidate stuck with all WAL sources unhealthy - #1143
Merged
Merged
Conversation
When a node is assigned the fast_forward goalstate it has not yet reported that state back to the monitor, so reportedstate is still report_lsn. If the originally-selected upstream peer transitions out of report_lsn before the fast_forward node calls get_most_advanced_standby(), the fast_forward node becomes the only remaining report_lsn node and is returned as its own upstream source. The node then loops forever trying to fetch WAL from itself. Fix across three layers: SQL (get_most_advanced_standby): add caller_node_id bigint parameter (default 0) and filter AND nodeid != $3 so the calling node can never be returned as its own upstream. monitor_get_most_advanced_standby: accept callerNodeId and a bool *found output parameter. A zero-row result is no longer an error: it means the caller is already the most advanced node and the found flag is set to false. keeper_get_most_advanced_standby: pass the local node ID to the monitor call; also skip self in the no-monitor path. Propagates the found flag to the caller. fsm_fast_forward: when found is false (no valid upstream), skip the WAL fetch and return true so the keeper can report its current state to the monitor. The monitor will then assign prepare_promotion on the next node_active call, breaking the loop. fsm_init_from_standby also uses keeper_get_most_advanced_standby; there a not-found result is a genuine error (no source to clone from) so it returns false with an explicit log message. Fixes #1060
…ealthy
When a failover candidate is assigned fast_forward (its LSN lags behind a
peer standby that holds more WAL), it fetches the missing WAL before
promotion. If the WAL-source node(s) die while the candidate is fetching,
the candidate gets stuck: it reports back report_lsn (failed fetch) while
its goal stays fast_forward. IsBeingPromoted() holds the lock, so the
election cannot restart, and ProceedWithMSFailover does nothing useful
because CandidateNodeIsReadyToStreamWAL explicitly excludes fast_forward.
Fix (group_state_machine.c):
Add WalSourceNodesAreAllUnhealthy() helper that scans the group for
nodes in {report_lsn, report_lsn} (the WAL-source state) whose health
is bad. When this fires for a candidate in {report_lsn, fast_forward}
inside ProceedGroupStateForMSFailover, the monitor acts based on the
guard_data_loss GUC:
- guard_data_loss=true (default): reset the candidate goal back to
report_lsn and WARN the operator. The election retries automatically
if a source recovers. Operators can unblock with:
pg_autoctl perform failover --allow-data-loss
- guard_data_loss=false: log and fall through.
get_most_advanced_standby() now filters out unhealthy nodes when
guard_data_loss=false (SQL change below), so fsm_fast_forward() finds
no upstream, skips the WAL fetch, and reports fast_forward as current.
The monitor then assigns prepare_promotion on the next call.
Fix (pgautofailover.sql):
get_most_advanced_standby() adds:
AND (current_setting('pgautofailover.guard_data_loss')::bool
OR health > 0)
When guard_data_loss=false, unhealthy nodes are excluded from WAL-source
selection. When guard_data_loss=true, the highest-LSN node is returned
regardless of health (preserving the no-data-loss guarantee).
Tests:
src/monitor/sql/fast_forward.sql - regression test: bootstraps a 3-node
formation, manually places the candidate in {report_lsn, fast_forward}
with the WAL source unhealthy, then:
Test A (guard_data_loss=true): node_active returns report_lsn ✓
Test B (guard_data_loss=false): node_active returns fast_forward ✓
Also calls get_most_advanced_standby() directly to exercise both SQL
filter paths.
tests/tap/specs/fast_forward.pgaf - integration test: kills primary and
WAL-source standby together, verifies node stays at report_lsn, then
unblocks with --allow-data-loss and verifies full cluster recovery.
src/monitor/Makefile and tests/tap/schedule updated accordingly.
Fixes #1060
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
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.
Problem
When a failover candidate is assigned
fast_forward(its LSN lags a peerstandby that holds more WAL), it must fetch the missing WAL before being
promoted. Two failure modes can produce a permanent liveness deadlock:
1. Self-reference loop (commit 1)
The monitor could select the old primary itself as the WAL source —
get_most_advanced_standbyhad no caller filter, so a node with the highestLSN in the group could be handed back to itself.
fsm_fast_forwardwouldthen loop forever trying to connect to its own endpoint.
2. Dead WAL source (commit 2)
If the WAL-source standby dies while the candidate is fetching, the candidate
reports back
report_lsn(failed fetch) while its goal staysfast_forward.IsBeingPromoted()locks the monitor intoProceedWithMSFailover, which doesnothing useful because
CandidateNodeIsReadyToStreamWALexplicitly excludesfast_forward. The election cannot restart; the cluster is stuck forever.Fix
Commit 1 — self-reference filter in
get_most_advanced_standbyAdd a
caller_node_id bigint default 0parameter to the SQL function andpropagate it from the keeper through
monitor_get_most_advanced_standbyandkeeper_get_most_advanced_standby. The SQL WHERE clause excludes the callingnode:
Commit 2 — stuck-detection + health filter
group_state_machine.c— newWalSourceNodesAreAllUnhealthy()helper scansthe group for nodes in
{report_lsn, report_lsn}(the WAL-source state).When every such peer is unhealthy and the candidate is in
{report_lsn, fast_forward}, the monitor acts based onpgautofailover.guard_data_loss:guard_data_loss = true(default): reset the candidate goal back toreport_lsnand WARN the operator. The election retries automatically if asource recovers. Operators can unblock immediately with:
guard_data_loss = false: log and fall through. The SQL health filter(below) excludes unhealthy sources, so
fsm_fast_forwardfinds no upstream,skips the WAL fetch, and reports
fast_forwardas its current state. Themonitor then assigns
prepare_promotionon the next call.pgautofailover.sql—get_most_advanced_standbyadds:When
guard_data_loss = false, unhealthy nodes are excluded from WAL-sourceselection, allowing the candidate to promote with available WAL. When
guard_data_loss = true, the highest-LSN node is returned regardless ofhealth, preserving the no-data-loss guarantee.
Tests
src/monitor/sql/fast_forward.sql— SQL regression test: bootstraps a3-node formation, manually places the candidate in
{report_lsn, fast_forward}with the WAL source unhealthy, then verifies:
guard_data_loss=true):node_activereturnsreport_lsn✓guard_data_loss=false):node_activereturnsfast_forward✓get_most_advanced_standbySQL filter both paths ✓tests/tap/specs/fast_forward.pgaf— integration test: kills primary andWAL-source standby together, verifies the surviving node stays at
report_lsn, then unblocks with--allow-data-lossand verifies fullcluster recovery to primary + secondary + secondary.
make installcheck).make docker-check).Fixes #1060