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_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/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..572353f2a --- /dev/null +++ b/tests/tap/specs/demote_timeout_wait_primary_deadlock.pgaf @@ -0,0 +1,84 @@ +# Reproduces https://github.com/hapostgres/pg_auto_failover/issues/1025. +# +# 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 + 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 + + # 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 + WHERE nodename = 'node2'; + } + 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; + } + + sleep 80s + network connect node1 + + # 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 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 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 + and node2 state is secondary + timeout 10s +}