Fix #1025: self-fenced primary deadlocks forever on an unreachable goal state

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.
 	 *

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

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)))
 		{

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
+}