From bc4c6fe8d5667f568fa7fbf8ab4d7d9c915c6cb8 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Sat, 25 Jul 2026 04:16:38 +0200 Subject: [PATCH 1/4] Fix #1025: self-fenced primary deadlocks forever on an unreachable goal 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. --- src/monitor/group_state_machine.c | 40 +++++++++ src/monitor/node_metadata.c | 15 +++- .../demote_timeout_wait_primary_deadlock.pgaf | 82 +++++++++++++++++++ 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf diff --git a/src/monitor/group_state_machine.c b/src/monitor/group_state_machine.c index 7ef662ac0..0ff353f77 100644 --- a/src/monitor/group_state_machine.c +++ b/src/monitor/group_state_machine.c @@ -189,6 +189,46 @@ ProceedGroupStateFromContext(GroupStateContext *ctx) return true; } + /* + * A node reporting demote_timeout may have gotten there on its own + * initiative (check_for_network_partitions() in service_keeper.c + * self-fences independently of whatever goal the monitor last assigned -- + * see #1025). If the currently assigned goal isn't one demote_timeout can + * actually reach, the keeper would fatal forever trying to get there. + * Re-target to demoted: always a valid demote_timeout exit + * (DEMOTE_TIMEOUT_STATE -> DEMOTED_STATE, fsm.c:355), and the safe, + * conservative choice -- the node stays fenced from writes until the + * existing "demoted -> catchingup" reintegration path + * (group_state_machine.c:909) or an operator decides otherwise. + * + * Deliberately a plain reportedState check, not IsCurrentState(): the + * whole point is to catch reportedState == demote_timeout while + * goalState is still whatever was assigned before the self-fence -- + * IsCurrentState() requires goalState == reportedState == state, which + * is exactly the case that does NOT need re-targeting (the node is + * already headed somewhere demote_timeout can reach). + */ + if (activeNode->reportedState == REPLICATION_STATE_DEMOTE_TIMEOUT && + activeNode->goalState != REPLICATION_STATE_DEMOTE_TIMEOUT && + activeNode->goalState != REPLICATION_STATE_DEMOTED && + activeNode->goalState != REPLICATION_STATE_PRIMARY && + activeNode->goalState != REPLICATION_STATE_SINGLE) + { + char message[BUFSIZE] = { 0 }; + + LogAndNotifyMessage( + message, BUFSIZE, + "Setting goal state of " NODE_FORMAT + " to demoted: it reports demote_timeout but is assigned %s, " + "which demote_timeout cannot reach.", + NODE_FORMAT_ARGS(activeNode), + ReplicationStateGetName(activeNode->goalState)); + + AssignGoalState(activeNode, REPLICATION_STATE_DEMOTED, message); + + return true; + } + /* * A node that is alone in its group should be SINGLE. * diff --git a/src/monitor/node_metadata.c b/src/monitor/node_metadata.c index 9c1660b7e..c6d4c0eae 100644 --- a/src/monitor/node_metadata.c +++ b/src/monitor/node_metadata.c @@ -557,7 +557,20 @@ GetPrimaryOrDemotedNodeInGroupFromList(List *groupNodeList) { AutoFailoverNode *currentNode = (AutoFailoverNode *) lfirst(nodeCell); - if (StateBelongsToPrimary(currentNode->reportedState) && + /* + * StateBelongsToPrimary() only covers the transitional states that + * lead up to a demotion (draining, demote_timeout, + * prepare_maintenance); it deliberately excludes the terminal + * "demoted" state itself, exactly like IsDemotedPrimary() below + * already accounts for. Without the explicit check here, a primary + * that fully converged to demoted (reportedState == goalState == + * demoted) with no other node ever promoted in its place -- e.g. a + * node recovering from a #1025 self-fence -- would not be found by + * either loop in this function, and callers relying on this + * function to always identify such a node would fail. + */ + if ((StateBelongsToPrimary(currentNode->reportedState) || + currentNode->reportedState == REPLICATION_STATE_DEMOTED) && (!IsBeingDemotedPrimary(primaryNode) || !IsDemotedPrimary(currentNode))) { diff --git a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf new file mode 100644 index 000000000..a9d83367c --- /dev/null +++ b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf @@ -0,0 +1,82 @@ +# Reproduces https://github.com/hapostgres/pg_auto_failover/issues/1025. +# +# node2 enters maintenance, a routine, non-failover event: the monitor +# assigns node1 (primary) the goal state "wait_primary" so it stops +# requiring a synchronous standby, and node2 the goal state +# "wait_maintenance". node1 has not yet locally converged to "wait_primary" +# (its current_role is still PRIMARY_STATE) when it loses contact with +# *both* the monitor and node2 for longer than network_partition_timeout +# (20s by default). +# +# check_for_network_partitions() in service_keeper.c only looks at node1's +# own current_role (still PRIMARY_STATE) to decide whether to self-fence; it +# has no idea the monitor already reassigned this node to "wait_primary" for +# an unrelated, benign reason. So it locally forces +# assigned_role = DEMOTE_TIMEOUT_STATE, Postgres is stopped, and node1 +# starts reporting "demote_timeout" -- without the monitor ever having been +# told. Once node1 reconnects, the monitor still has goalstate = +# "wait_primary" from before, and the keeper FSM has no +# demote_timeout -> wait_primary edge, so node1 fatals forever with: +# +# FATAL pg_autoctl does not know how to reach state +# "wait_primary" from "demote_timeout" +# +# and node2 is stuck in wait_maintenance waiting for node1 to converge. +# +# Fixed by a guard in ProceedGroupStateFromContext (group_state_machine.c): +# when a node reports demote_timeout but its assigned goal isn't one +# demote_timeout can actually reach, the monitor re-targets it to demoted -- +# a real demote_timeout -> demoted FSM edge (fsm.c:355) -- instead of +# leaving it assigned an unreachable goal forever. +# +# demoted is a safe, conservative landing spot (the node stays fenced from +# writes) but not a self-healing one: with no failover ever triggered, node2 +# never became a promotion candidate (report_lsn), so there is no existing +# monitor mechanism (perform_failover included -- see +# GetNodeToFailoverFromInGroup) 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; this spec only covers the #1025 deadlock +# itself, i.e. that node1 no longer fatals forever. + +cluster { + monitor + ssl off + formation { + node1 + node2 + } +} + +setup { + wait until primary, secondary timeout 120s +} + +teardown { + compose down +} + +step test_001_1025_self_fence_recovers_instead_of_deadlocking { + wait until node1 state is primary + and node2 state is secondary + timeout 60s + network disconnect node1 + sql monitor { + SELECT pgautofailover.start_maintenance(nodeid) + FROM pgautofailover.node + WHERE nodename = 'node2'; + } + wait until node1 assigned-state = wait_primary timeout 60s + sleep 90s + network connect node1 + + # Before the fix: the monitor still has goalstate = wait_primary from + # before the self-fence -- a state demote_timeout has no FSM edge to -- + # so pg_autoctl fatals forever. After the fix: as soon as node1 reports + # demote_timeout, the monitor's guard notices the assigned goalState is + # unreachable from there and re-targets to demoted in that same + # node_active() call -- so goalState jumps straight from wait_primary to + # demoted without ever stably sitting at demote_timeout, and only the + # fully-converged "demoted" state is observable here. + wait until node1 state is demoted timeout 60s + logs node1 not contains "does not know how to reach state" +} From 6b75ff77f2049cb6bd2a9e07ed4fddc87c55b418 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Sat, 25 Jul 2026 15:22:48 +0200 Subject: [PATCH 2/4] Extend #1025 spec: document the secondary's state and the recovery dead 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. --- .../demote_timeout_wait_primary_deadlock.pgaf | 120 ++++++++++++++++-- 1 file changed, 112 insertions(+), 8 deletions(-) diff --git a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf index a9d83367c..e64e0e644 100644 --- a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf +++ b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf @@ -30,13 +30,7 @@ # leaving it assigned an unreachable goal forever. # # demoted is a safe, conservative landing spot (the node stays fenced from -# writes) but not a self-healing one: with no failover ever triggered, node2 -# never became a promotion candidate (report_lsn), so there is no existing -# monitor mechanism (perform_failover included -- see -# GetNodeToFailoverFromInGroup) 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; this spec only covers the #1025 deadlock -# itself, i.e. that node1 no longer fatals forever. +# writes) but, as steps 002 and 003 below show, not a self-healing one. cluster { monitor @@ -66,7 +60,25 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { WHERE nodename = 'node2'; } wait until node1 assigned-state = wait_primary timeout 60s - sleep 90s + + # While node1 is disconnected: this is the monitor's own view of it -- + # the last state it successfully reported before going silent (still + # "primary": the monitor doesn't yet know a self-fence is about to happen + # locally on node1), an assigned goal of "wait_primary" from + # start_maintenance() above, and health that will degrade once the + # monitor's own health checks start timing out. node2 may still be mid + # transition to "maintenance" at this exact instant (stopping its own + # replication takes a moment) -- either way, it needs nothing further + # from node1 to get there. + sleep 10s + sql monitor { + SELECT nodename, reportedstate, goalstate, health + FROM pgautofailover.node ORDER BY nodeid; + } + + # Let the self-fence complete locally on node1 (demote_timeout, Postgres + # stopped: network_partition_timeout defaults to 20s), then reconnect it. + sleep 80s network connect node1 # Before the fix: the monitor still has goalstate = wait_primary from @@ -80,3 +92,95 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { wait until node1 state is demoted timeout 60s logs node1 not contains "does not know how to reach state" } + +# What happened to node2 (the former secondary) throughout all of this, and +# what state is it in now that node1 has converged to demoted? +# +# Untouched, the whole way through. node2's entry into "maintenance" is +# fully independent of node1: stop_replication/maintenance only requires +# node2 to disconnect from its own upstream and tell the monitor, which +# needs no cooperation from node1. Once node2 reports "maintenance", the +# MAINTENANCE early-exit at the top of ProceedGroupStateFromContext +# (group_state_machine.c:187-190) means the monitor will not touch its goal +# state again until an explicit stop_maintenance() call -- regardless of +# anything happening to node1 in the meantime. node2 has been sitting there +# since test_001's start_maintenance() call, completely unaware that node1 +# ever disconnected, self-fenced, or came back as demoted. +step test_002_secondary_is_unaffected_by_primarys_self_fence { + wait until node2 state is maintenance timeout 30s + assert node2 assigned-state = maintenance + + # Full picture at this point: node1 fully converged to demoted (fenced, + # Postgres stopped, no writes possible anywhere), node2 still parked in + # maintenance exactly where test_001 left it. Two independent islands. + sql monitor { + SELECT nodename, reportedstate, goalstate, health + FROM pgautofailover.node ORDER BY nodeid; + } +} + +# How do we repair the cluster and converge back to a working primary? +# +# With the tools that exist today: we can't, not without further operator +# intervention beyond what the monitor offers. This step demonstrates the +# dead end precisely so it's not lost knowledge: +# +# 1. stop_maintenance(node2) is the obvious next move -- but at the moment +# it's called, IsFailoverInProgress() (node_metadata.c:657) is false: +# nothing is in report_lsn/join_secondary yet, and node1 being "demoted" +# alone doesn't count. So stop_maintenance() takes the plain +# "rejoin the existing primary" branch and assigns node2 the goal +# "catchingup" (node_active_protocol.c:2119-2128) -- meaning "stream from +# whichever node is currently primary". But node1 is demoted, not +# primary, and its Postgres is stopped: node2 retries the replication +# connection to node1 forever and never reaches "secondary". +# +# 2. pgautofailover.perform_failover() can't rescue it either: +# GetNodeToFailoverFromInGroup() only considers a node a valid failover +# source when CanInitiateFailover(goalState) is true (single, primary, or +# join_primary -- node_metadata.c:2056-2063); "demoted" isn't one of +# them, and no node is in report_lsn to fall back on, so it errors out +# with "couldn't find the primary node". +# +# 3. pgautofailover.perform_promotion() can't either: it requires its target +# node to be in "secondary" or "report_lsn" (node_active_protocol.c: +# 1668-1682); node2 is stuck in "catchingup" by this point, so it errors +# out with "promotion can only be performed when in state secondary". +# +# The underlying gap: a demoted primary only ever gets swept into the +# report_lsn candidate pool by the candidate-list-building code inside +# ProceedGroupStateForMSFailover() (group_state_machine.c:~1879), but that +# function is only entered once a failover is already considered "in +# progress" -- which itself requires some node to already be in report_lsn. +# Nothing in this formation ever puts a node into report_lsn in the first +# place, so the group is stuck: demoted primary, no candidate, no automatic +# or documented manual path back. This is a separate, pre-existing gap from +# #1025 (which is only about the deadlock/fatal-crash-loop, now fixed) -- +# recovering formations like this one is unsolved and needs its own fix. +step test_003_no_existing_mechanism_converges_the_cluster_back { + sql monitor { + SELECT pgautofailover.stop_maintenance(nodeid) + FROM pgautofailover.node + WHERE nodename = 'node2'; + } + wait until node2 assigned-state = catchingup timeout 30s + + # Give node2 a few retry cycles against node1's stopped Postgres. + sleep 20s + logs node2 contains "Connection refused" + + sql monitor { + SELECT nodename, reportedstate, goalstate, health + FROM pgautofailover.node ORDER BY nodeid; + } + + sql monitor { + SELECT pgautofailover.perform_failover('default', 0); + } + expect error + + sql monitor { + SELECT pgautofailover.perform_promotion('default', 'node2'); + } + expect error +} From fdbb462592381a386925ef73f355f0a6eeaa7d58 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Sat, 25 Jul 2026 15:46:19 +0200 Subject: [PATCH 3/4] Recover formations stuck with a demoted primary and no candidate 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. --- src/monitor/node_active_protocol.c | 26 ++- .../demote_timeout_wait_primary_deadlock.pgaf | 170 +++++------------- 2 files changed, 72 insertions(+), 124 deletions(-) diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index 459514e92..24a0d0163 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -2087,7 +2087,8 @@ stop_maintenance(PG_FUNCTION_ARGS) "group %d", currentNode->formationId, currentNode->groupId))); } - else if (primaryNode == NULL && totalNodesCount > 2) + else if ((primaryNode == NULL || IsDemotedPrimary(primaryNode)) && + totalNodesCount > 2) { LogAndNotifyMessage( message, BUFSIZE, @@ -2099,6 +2100,29 @@ stop_maintenance(PG_FUNCTION_ARGS) PG_RETURN_BOOL(true); } + else if (IsDemotedPrimary(primaryNode)) + { + /* + * The primary is fully demoted (Postgres stopped, e.g. after a + * #1025 self-fence recovery): there's nothing left running to + * stream from, so catchingup would just retry a doomed replication + * connection forever. Join the report_lsn crew instead -- once this + * node reports its LSN, the candidate-scanning code in + * ProceedGroupStateForMSFailover() picks up the demoted primary too + * (it's still IsDemotedPrimary()) and the normal election proceeds. + */ + LogAndNotifyMessage( + message, BUFSIZE, + "Setting goal state of " NODE_FORMAT + " to report_lsn after a user-initiated stop_maintenance call, " + "as " NODE_FORMAT " is demoted and has nothing to catch up from.", + NODE_FORMAT_ARGS(currentNode), + NODE_FORMAT_ARGS(primaryNode)); + + SetNodeGoalState(currentNode, REPLICATION_STATE_REPORT_LSN, message); + + PG_RETURN_BOOL(true); + } /* * When a failover is in progress and stop_maintenance() is called (by diff --git a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf index e64e0e644..9d3611619 100644 --- a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf +++ b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf @@ -1,36 +1,20 @@ # Reproduces https://github.com/hapostgres/pg_auto_failover/issues/1025. # -# node2 enters maintenance, a routine, non-failover event: the monitor -# assigns node1 (primary) the goal state "wait_primary" so it stops -# requiring a synchronous standby, and node2 the goal state -# "wait_maintenance". node1 has not yet locally converged to "wait_primary" -# (its current_role is still PRIMARY_STATE) when it loses contact with -# *both* the monitor and node2 for longer than network_partition_timeout -# (20s by default). -# -# check_for_network_partitions() in service_keeper.c only looks at node1's -# own current_role (still PRIMARY_STATE) to decide whether to self-fence; it -# has no idea the monitor already reassigned this node to "wait_primary" for -# an unrelated, benign reason. So it locally forces -# assigned_role = DEMOTE_TIMEOUT_STATE, Postgres is stopped, and node1 -# starts reporting "demote_timeout" -- without the monitor ever having been -# told. Once node1 reconnects, the monitor still has goalstate = -# "wait_primary" from before, and the keeper FSM has no -# demote_timeout -> wait_primary edge, so node1 fatals forever with: +# node2 enters maintenance (benign, no failover): monitor assigns node1 +# (primary) goal wait_primary so it stops requiring a sync standby. node1 +# hasn't locally converged to that yet when it loses contact with the +# monitor and node2 for longer than network_partition_timeout (20s default). +# check_for_network_partitions() (service_keeper.c) only looks at node1's own +# current_role, has no idea the monitor reassigned it, and self-fences to +# demote_timeout. The monitor still has goalstate = wait_primary, a state +# demote_timeout has no FSM edge to, so node1 used to fatal forever: # # FATAL pg_autoctl does not know how to reach state # "wait_primary" from "demote_timeout" # -# and node2 is stuck in wait_maintenance waiting for node1 to converge. -# # Fixed by a guard in ProceedGroupStateFromContext (group_state_machine.c): -# when a node reports demote_timeout but its assigned goal isn't one -# demote_timeout can actually reach, the monitor re-targets it to demoted -- -# a real demote_timeout -> demoted FSM edge (fsm.c:355) -- instead of -# leaving it assigned an unreachable goal forever. -# -# demoted is a safe, conservative landing spot (the node stays fenced from -# writes) but, as steps 002 and 003 below show, not a self-healing one. +# a node reporting demote_timeout with an unreachable goal gets re-targeted +# to demoted (a real demote_timeout -> demoted FSM edge, fsm.c:355). cluster { monitor @@ -54,6 +38,11 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { and node2 state is secondary timeout 60s network disconnect node1 + + # Not `pg_autoctl enable maintenance`: that CLI blocks until node2 + # reaches "maintenance", which can't happen while node1 is disconnected + # (see test_002) -- the whole point here is node1 stays disconnected + # while node2's request is only assigned, not yet converged. sql monitor { SELECT pgautofailover.start_maintenance(nodeid) FROM pgautofailover.node @@ -61,126 +50,61 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { } wait until node1 assigned-state = wait_primary timeout 60s - # While node1 is disconnected: this is the monitor's own view of it -- - # the last state it successfully reported before going silent (still - # "primary": the monitor doesn't yet know a self-fence is about to happen - # locally on node1), an assigned goal of "wait_primary" from - # start_maintenance() above, and health that will degrade once the - # monitor's own health checks start timing out. node2 may still be mid - # transition to "maintenance" at this exact instant (stopping its own - # replication takes a moment) -- either way, it needs nothing further - # from node1 to get there. sleep 10s sql monitor { SELECT nodename, reportedstate, goalstate, health FROM pgautofailover.node ORDER BY nodeid; } - # Let the self-fence complete locally on node1 (demote_timeout, Postgres - # stopped: network_partition_timeout defaults to 20s), then reconnect it. sleep 80s network connect node1 - # Before the fix: the monitor still has goalstate = wait_primary from - # before the self-fence -- a state demote_timeout has no FSM edge to -- - # so pg_autoctl fatals forever. After the fix: as soon as node1 reports - # demote_timeout, the monitor's guard notices the assigned goalState is - # unreachable from there and re-targets to demoted in that same - # node_active() call -- so goalState jumps straight from wait_primary to - # demoted without ever stably sitting at demote_timeout, and only the - # fully-converged "demoted" state is observable here. + # goalState jumps wait_primary -> demoted in one node_active() call, so + # "demote_timeout" itself is never observable here. wait until node1 state is demoted timeout 60s logs node1 not contains "does not know how to reach state" } -# What happened to node2 (the former secondary) throughout all of this, and -# what state is it in now that node1 has converged to demoted? +# node2 is stuck at wait_maintenance (not maintenance) for the entire +# disconnect: group_state_machine.c:664 only promotes wait_maintenance -> +# maintenance once the primary's *reported* state is wait_primary, or +# (:689) once the primary's *goal* moves off wait_primary. Neither happens +# while node1 is unreachable with goal stuck at wait_primary. The moment the +# #1025 guard re-targets node1's goal to demoted, rule :689 fires and node2 +# reaches maintenance in the same beat -- which is why test_001 saw node1 +# converge to demoted and node2 to maintenance together. # -# Untouched, the whole way through. node2's entry into "maintenance" is -# fully independent of node1: stop_replication/maintenance only requires -# node2 to disconnect from its own upstream and tell the monitor, which -# needs no cooperation from node1. Once node2 reports "maintenance", the -# MAINTENANCE early-exit at the top of ProceedGroupStateFromContext -# (group_state_machine.c:187-190) means the monitor will not touch its goal -# state again until an explicit stop_maintenance() call -- regardless of -# anything happening to node1 in the meantime. node2 has been sitting there -# since test_001's start_maintenance() call, completely unaware that node1 -# ever disconnected, self-fenced, or came back as demoted. +# From here on node2 is independent: the MAINTENANCE early-exit in +# ProceedGroupStateFromContext (line 187) holds its goal state until an +# explicit disable maintenance call. step test_002_secondary_is_unaffected_by_primarys_self_fence { wait until node2 state is maintenance timeout 30s assert node2 assigned-state = maintenance - - # Full picture at this point: node1 fully converged to demoted (fenced, - # Postgres stopped, no writes possible anywhere), node2 still parked in - # maintenance exactly where test_001 left it. Two independent islands. sql monitor { SELECT nodename, reportedstate, goalstate, health FROM pgautofailover.node ORDER BY nodeid; } } -# How do we repair the cluster and converge back to a working primary? -# -# With the tools that exist today: we can't, not without further operator -# intervention beyond what the monitor offers. This step demonstrates the -# dead end precisely so it's not lost knowledge: -# -# 1. stop_maintenance(node2) is the obvious next move -- but at the moment -# it's called, IsFailoverInProgress() (node_metadata.c:657) is false: -# nothing is in report_lsn/join_secondary yet, and node1 being "demoted" -# alone doesn't count. So stop_maintenance() takes the plain -# "rejoin the existing primary" branch and assigns node2 the goal -# "catchingup" (node_active_protocol.c:2119-2128) -- meaning "stream from -# whichever node is currently primary". But node1 is demoted, not -# primary, and its Postgres is stopped: node2 retries the replication -# connection to node1 forever and never reaches "secondary". +# Repair: disable maintenance on node2 while node1 is demoted. # -# 2. pgautofailover.perform_failover() can't rescue it either: -# GetNodeToFailoverFromInGroup() only considers a node a valid failover -# source when CanInitiateFailover(goalState) is true (single, primary, or -# join_primary -- node_metadata.c:2056-2063); "demoted" isn't one of -# them, and no node is in report_lsn to fall back on, so it errors out -# with "couldn't find the primary node". +# stop_maintenance() finds node1 via GetPrimaryOrDemotedNodeInGroupFromList() +# -- recognizing a fully-converged demoted node is itself a #1025 fix +# prerequisite. Seeing node1 is demoted (not actually running), +# stop_maintenance() now assigns node2 report_lsn instead of catchingup: +# catchingup would mean "stream from node1", but node1's Postgres is +# stopped, so that used to retry a doomed connection forever. # -# 3. pgautofailover.perform_promotion() can't either: it requires its target -# node to be in "secondary" or "report_lsn" (node_active_protocol.c: -# 1668-1682); node2 is stuck in "catchingup" by this point, so it errors -# out with "promotion can only be performed when in state secondary". -# -# The underlying gap: a demoted primary only ever gets swept into the -# report_lsn candidate pool by the candidate-list-building code inside -# ProceedGroupStateForMSFailover() (group_state_machine.c:~1879), but that -# function is only entered once a failover is already considered "in -# progress" -- which itself requires some node to already be in report_lsn. -# Nothing in this formation ever puts a node into report_lsn in the first -# place, so the group is stuck: demoted primary, no candidate, no automatic -# or documented manual path back. This is a separate, pre-existing gap from -# #1025 (which is only about the deadlock/fatal-crash-loop, now fixed) -- -# recovering formations like this one is unsolved and needs its own fix. -step test_003_no_existing_mechanism_converges_the_cluster_back { - sql monitor { - SELECT pgautofailover.stop_maintenance(nodeid) - FROM pgautofailover.node - WHERE nodename = 'node2'; - } - wait until node2 assigned-state = catchingup timeout 30s - - # Give node2 a few retry cycles against node1's stopped Postgres. - sleep 20s - logs node2 contains "Connection refused" - - sql monitor { - SELECT nodename, reportedstate, goalstate, health - FROM pgautofailover.node ORDER BY nodeid; - } - - sql monitor { - SELECT pgautofailover.perform_failover('default', 0); - } - expect error - - sql monitor { - SELECT pgautofailover.perform_promotion('default', 'node2'); - } - expect error +# node2 reaching report_lsn is what seeds the election: +# ProceedGroupStateForMSFailover()'s candidate scan (~line 1879) then also +# picks up node1 (IsDemotedPrimary() matches it), both report their LSN, and +# the most advanced one wins -- node1, since node2 stopped replicating when +# it went into maintenance. `pg_autoctl disable maintenance` blocks on the +# monitor until node2 itself reaches secondary or primary, so by the time it +# returns the election is already settled. +step test_003_disable_maintenance_converges_the_cluster_back { + exec node2 pg_autoctl disable maintenance + wait until node1 state is primary + and node2 state is secondary + timeout 10s } From 6e572339e495993c59186b4c4cde87e72ccc612c Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Sat, 25 Jul 2026 16:01:59 +0200 Subject: [PATCH 4/4] Trim comments in the #1025 spec, label debug-only SQL snapshots 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. --- .../demote_timeout_wait_primary_deadlock.pgaf | 70 ++++++------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf index 9d3611619..572353f2a 100644 --- a/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf +++ b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf @@ -1,20 +1,10 @@ # Reproduces https://github.com/hapostgres/pg_auto_failover/issues/1025. # -# node2 enters maintenance (benign, no failover): monitor assigns node1 -# (primary) goal wait_primary so it stops requiring a sync standby. node1 -# hasn't locally converged to that yet when it loses contact with the -# monitor and node2 for longer than network_partition_timeout (20s default). -# check_for_network_partitions() (service_keeper.c) only looks at node1's own -# current_role, has no idea the monitor reassigned it, and self-fences to -# demote_timeout. The monitor still has goalstate = wait_primary, a state -# demote_timeout has no FSM edge to, so node1 used to fatal forever: -# -# FATAL pg_autoctl does not know how to reach state -# "wait_primary" from "demote_timeout" -# -# Fixed by a guard in ProceedGroupStateFromContext (group_state_machine.c): -# a node reporting demote_timeout with an unreachable goal gets re-targeted -# to demoted (a real demote_timeout -> demoted FSM edge, fsm.c:355). +# node2 enters maintenance while node1 (primary) is network-partitioned. +# node1 self-fences to demote_timeout without the monitor knowing, and used +# to fatal forever trying to reach its stale goal state (wait_primary). +# Fixed by a guard in ProceedGroupStateFromContext that re-targets an +# unreachable demote_timeout goal to demoted. cluster { monitor @@ -39,10 +29,8 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { timeout 60s network disconnect node1 - # Not `pg_autoctl enable maintenance`: that CLI blocks until node2 - # reaches "maintenance", which can't happen while node1 is disconnected - # (see test_002) -- the whole point here is node1 stays disconnected - # while node2's request is only assigned, not yet converged. + # Raw SQL, not `pg_autoctl enable maintenance`: that CLI blocks until + # node2 reaches "maintenance", which can't happen while node1 is down. sql monitor { SELECT pgautofailover.start_maintenance(nodeid) FROM pgautofailover.node @@ -51,6 +39,10 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { wait until node1 assigned-state = wait_primary timeout 60s sleep 10s + + # DEBUG: no expect, just a snapshot of the monitor's view mid-disconnect + # in the test output (node1 still "primary", last report before going + # silent). sql monitor { SELECT nodename, reportedstate, goalstate, health FROM pgautofailover.node ORDER BY nodeid; @@ -59,49 +51,31 @@ step test_001_1025_self_fence_recovers_instead_of_deadlocking { sleep 80s network connect node1 - # goalState jumps wait_primary -> demoted in one node_active() call, so + # goalState jumps wait_primary -> demoted in one call, so # "demote_timeout" itself is never observable here. wait until node1 state is demoted timeout 60s logs node1 not contains "does not know how to reach state" } -# node2 is stuck at wait_maintenance (not maintenance) for the entire -# disconnect: group_state_machine.c:664 only promotes wait_maintenance -> -# maintenance once the primary's *reported* state is wait_primary, or -# (:689) once the primary's *goal* moves off wait_primary. Neither happens -# while node1 is unreachable with goal stuck at wait_primary. The moment the -# #1025 guard re-targets node1's goal to demoted, rule :689 fires and node2 -# reaches maintenance in the same beat -- which is why test_001 saw node1 -# converge to demoted and node2 to maintenance together. -# -# From here on node2 is independent: the MAINTENANCE early-exit in -# ProceedGroupStateFromContext (line 187) holds its goal state until an -# explicit disable maintenance call. +# node2 stays at wait_maintenance the whole time node1 is down, and only +# reaches maintenance once node1's goal moves off wait_primary (here: the +# moment the #1025 guard re-targets it to demoted). step test_002_secondary_is_unaffected_by_primarys_self_fence { wait until node2 state is maintenance timeout 30s assert node2 assigned-state = maintenance + + # DEBUG: no expect, just a snapshot -- node1 demoted, node2 maintenance. sql monitor { SELECT nodename, reportedstate, goalstate, health FROM pgautofailover.node ORDER BY nodeid; } } -# Repair: disable maintenance on node2 while node1 is demoted. -# -# stop_maintenance() finds node1 via GetPrimaryOrDemotedNodeInGroupFromList() -# -- recognizing a fully-converged demoted node is itself a #1025 fix -# prerequisite. Seeing node1 is demoted (not actually running), -# stop_maintenance() now assigns node2 report_lsn instead of catchingup: -# catchingup would mean "stream from node1", but node1's Postgres is -# stopped, so that used to retry a doomed connection forever. -# -# node2 reaching report_lsn is what seeds the election: -# ProceedGroupStateForMSFailover()'s candidate scan (~line 1879) then also -# picks up node1 (IsDemotedPrimary() matches it), both report their LSN, and -# the most advanced one wins -- node1, since node2 stopped replicating when -# it went into maintenance. `pg_autoctl disable maintenance` blocks on the -# monitor until node2 itself reaches secondary or primary, so by the time it -# returns the election is already settled. +# Repair: disable maintenance while node1 is demoted. stop_maintenance() now +# recognizes a fully-demoted primary and assigns node2 report_lsn instead of +# catchingup (nothing to stream from), which seeds a normal candidate +# election that also picks up node1 -- and node1 wins, since node2 stopped +# replicating when it went into maintenance. step test_003_disable_maintenance_converges_the_cluster_back { exec node2 pg_autoctl disable maintenance wait until node1 state is primary