diff --git a/src/bin/pg_autoctl/fsm_transition.c b/src/bin/pg_autoctl/fsm_transition.c index bc768cb04..25698c790 100644 --- a/src/bin/pg_autoctl/fsm_transition.c +++ b/src/bin/pg_autoctl/fsm_transition.c @@ -1320,17 +1320,31 @@ fsm_fast_forward(Keeper *keeper) ReplicationSource *upstream = &(postgres->replicationSource); NodeAddress upstreamNode = { 0 }; + bool found = false; char slotName[MAXCONNINFO] = { 0 }; - /* get the primary node to follow */ - if (!keeper_get_most_advanced_standby(keeper, &upstreamNode)) + /* get the most advanced peer standby to fetch missing WAL from */ + if (!keeper_get_most_advanced_standby(keeper, &upstreamNode, &found)) { log_error("Failed to fast forward from the most advanced standby node, " "see above for details"); return false; } + /* + * When no other report_lsn peer exists (because the intended upstream + * already transitioned away), this node is now the most advanced. + * Skip the WAL fetch — the monitor will assign prepare_promotion on the + * next node_active call. + */ + if (!found) + { + log_info("No upstream standby found for fast_forward; " + "skipping WAL fetch and proceeding to promotion"); + return true; + } + /* * Postgres 10 does not have pg_replication_slot_advance(), so we don't * support replication slots on standby nodes there. @@ -1471,15 +1485,23 @@ fsm_init_from_standby(Keeper *keeper) LocalPostgresServer *postgres = &(keeper->postgres); NodeAddress upstreamNode = { 0 }; + bool found = false; /* get the primary node to follow */ - if (!keeper_get_most_advanced_standby(keeper, &upstreamNode)) + if (!keeper_get_most_advanced_standby(keeper, &upstreamNode, &found)) { log_error("Failed to initialise from the most advanced standby node, " "see above for details"); return false; } + if (!found) + { + log_error("No standby node found to initialise from; " + "cannot proceed without an upstream source"); + return false; + } + if (!standby_init_replication_source(postgres, &upstreamNode, PG_AUTOCTL_REPLICA_USERNAME, diff --git a/src/bin/pg_autoctl/keeper.c b/src/bin/pg_autoctl/keeper.c index d0cb4e351..8ec85e9d9 100644 --- a/src/bin/pg_autoctl/keeper.c +++ b/src/bin/pg_autoctl/keeper.c @@ -3109,10 +3109,12 @@ keeper_get_primary(Keeper *keeper, NodeAddress *primaryNode) * the keeper->otherNodes array. */ bool -keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *upstreamNode) +keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *upstreamNode, + bool *found) { KeeperConfig *config = &(keeper->config); int groupId = keeper->state.current_group; + int64_t localNodeId = keeper->state.current_node_id; if (!config->monitorDisabled) { @@ -3121,7 +3123,9 @@ keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *upstreamNode) if (!monitor_get_most_advanced_standby(monitor, config->formation, groupId, - upstreamNode)) + localNodeId, + upstreamNode, + found)) { log_error("Failed to get the most advanced standby node " "from the monitor, see above for details"); @@ -3140,6 +3144,12 @@ keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *upstreamNode) NodeAddress *node = &(keeper->otherNodes.nodes[i]); uint64_t nodeLSN = 0; + /* skip self to avoid fetching WAL from ourselves */ + if (node->nodeId == localNodeId) + { + continue; + } + if (!parseLSN(node->lsn, &nodeLSN)) { log_error("Failed to parse node %" PRId64 @@ -3158,14 +3168,14 @@ keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *upstreamNode) if (mostAdvandedStandbyNode == NULL) { - log_error("Failed to get the most avdanced standby node " - "from the current list of other nodes, " - "refresh the list with the command: " - "pg_autoctl do fsm nodes set"); - return false; + log_info("No other standby found in local node list; " + "node %" PRId64 " is the most advanced", localNodeId); + *found = false; + return true; } *upstreamNode = *mostAdvandedStandbyNode; + *found = true; return true; } diff --git a/src/bin/pg_autoctl/keeper.h b/src/bin/pg_autoctl/keeper.h index 0b2a61345..15c0729d1 100644 --- a/src/bin/pg_autoctl/keeper.h +++ b/src/bin/pg_autoctl/keeper.h @@ -120,7 +120,8 @@ bool keeper_refresh_citus_remove_dropped_nodes(Keeper *keeper, bool keeper_read_nodes_from_file(Keeper *keeper, NodeAddressArray *nodesArray); bool keeper_get_primary(Keeper *keeper, NodeAddress *primaryNode); -bool keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *primaryNode); +bool keeper_get_most_advanced_standby(Keeper *keeper, NodeAddress *primaryNode, + bool *found); bool keeper_pg_autoctl_get_version_from_disk(Keeper *keeper, diff --git a/src/bin/pg_autoctl/monitor.c b/src/bin/pg_autoctl/monitor.c index 7ceec151e..6a3983d7c 100644 --- a/src/bin/pg_autoctl/monitor.c +++ b/src/bin/pg_autoctl/monitor.c @@ -751,23 +751,26 @@ monitor_get_coordinator(Monitor *monitor, char *formation, bool monitor_get_most_advanced_standby(Monitor *monitor, char *formation, int groupId, - NodeAddress *node) + int64_t callerNodeId, + NodeAddress *node, bool *found) { PGSQL *pgsql = &monitor->pgsql; const char *sql = - "SELECT * FROM pgautofailover.get_most_advanced_standby($1, $2)"; - int paramCount = 2; - Oid paramTypes[2] = { TEXTOID, INT4OID }; - const char *paramValues[2]; + "SELECT * FROM pgautofailover.get_most_advanced_standby($1, $2, $3)"; + int paramCount = 3; + Oid paramTypes[3] = { TEXTOID, INT4OID, INT8OID }; + const char *paramValues[3]; - /* we expect a single entry */ + /* we expect zero or one entry */ NodeAddressArray nodeArray = { 0 }; NodeAddressArrayParseContext parseContext = { { 0 }, &nodeArray, false }; IntString groupIdString = intToString(groupId); + IntString callerNodeIdString = intToString(callerNodeId); paramValues[0] = formation; paramValues[1] = groupIdString.strValue; + paramValues[2] = callerNodeIdString.strValue; if (!pgsql_execute_with_params(pgsql, sql, paramCount, paramTypes, paramValues, @@ -781,7 +784,7 @@ monitor_get_most_advanced_standby(Monitor *monitor, return false; } - if (!parseContext.parsedOK || nodeArray.count != 1) + if (!parseContext.parsedOK) { log_error( "Failed to get the most advanced standby node from the monitor " @@ -792,6 +795,15 @@ monitor_get_most_advanced_standby(Monitor *monitor, return false; } + /* zero rows: no other report_lsn peer exists; caller is most advanced */ + if (nodeArray.count == 0) + { + log_info("No other standby is reporting its LSN; " + "node %" PRId64 " is the most advanced", callerNodeId); + *found = false; + return true; + } + /* copy the node we retrieved in the expected place */ node->nodeId = nodeArray.nodes[0].nodeId; strlcpy(node->name, nodeArray.nodes[0].name, _POSIX_HOST_NAME_MAX); @@ -803,6 +815,7 @@ monitor_get_most_advanced_standby(Monitor *monitor, log_debug("The most advanced standby node is node " NODE_FORMAT, node->nodeId, node->name, node->host, node->port); + *found = true; return true; } diff --git a/src/bin/pg_autoctl/monitor.h b/src/bin/pg_autoctl/monitor.h index aea12ae2e..d18bf16cf 100644 --- a/src/bin/pg_autoctl/monitor.h +++ b/src/bin/pg_autoctl/monitor.h @@ -124,7 +124,8 @@ bool monitor_get_coordinator(Monitor *monitor, char *formation, CoordinatorNodeAddress *coordinatorNodeAddress); bool monitor_get_most_advanced_standby(Monitor *monitor, char *formation, int groupId, - NodeAddress *node); + int64_t callerNodeId, + NodeAddress *node, bool *found); bool monitor_register_node(Monitor *monitor, char *formation, char *name, diff --git a/src/monitor/Makefile b/src/monitor/Makefile index eca708ccf..7e2193186 100644 --- a/src/monitor/Makefile +++ b/src/monitor/Makefile @@ -14,7 +14,7 @@ MODULE_big = $(EXTENSION) OBJS = $(patsubst ${SRC_DIR}%.c,%.o,$(wildcard ${SRC_DIR}*.c)) PG_CPPFLAGS = -std=c99 -Wall -Werror -Wno-unused-parameter -Iinclude -I$(libpq_srcdir) -g SHLIB_LINK = $(libpq) -REGRESS = create_extension monitor workers node_active_protocol guard_data_loss dummy_update drop_extension upgrade +REGRESS = create_extension monitor workers node_active_protocol guard_data_loss fast_forward dummy_update drop_extension upgrade PG_CONFIG ?= pg_config PGXS = $(shell $(PG_CONFIG) --pgxs) diff --git a/src/monitor/expected/fast_forward.out b/src/monitor/expected/fast_forward.out new file mode 100644 index 000000000..e39f554ed --- /dev/null +++ b/src/monitor/expected/fast_forward.out @@ -0,0 +1,387 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression tests for the fast_forward WAL-fetch stuck detection introduced +-- in group_state_machine.c (ProceedGroupStateForMSFailover). +-- +-- Scenario: 3-node formation (p + s1 + s2, number_sync_standbys=1). +-- p dies → draining. +-- s1 has the higher LSN (0/5100) → stays in report_lsn (WAL source). +-- s2 has a lower LSN (0/5000) but higher candidate_priority → selected as +-- fast_forward candidate. +-- s1 then becomes unhealthy → s2's WAL fetch has no source. +-- +-- Test A (guard_data_loss=true): +-- node_active for s2 with reportedState=report_lsn resets s2 goal to +-- report_lsn (stuck-fast_forward guard fires). +-- +-- Test B (guard_data_loss=false): +-- node_active for s2 with reportedState=report_lsn falls through; s2 goal +-- stays fast_forward. +-- +-- Also exercises get_most_advanced_standby() directly: +-- guard_data_loss=true → returns s1 (unhealthy but included by SQL filter). +-- guard_data_loss=false → returns no rows (unhealthy nodes filtered out, +-- s2 excluded by caller_node_id). +\x on +-- ── formation setup ────────────────────────────────────────────────────────── +SELECT pgautofailover.create_formation('ff_test', 'pgsql', 'postgres', true, 1); +-[ RECORD 1 ]----+----------------------------- +create_formation | (ff_test,pgsql,postgres,t,1) + +-- Register three nodes. +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_p', 5432, + 'postgres', 'ff_p', 1); +-[ RECORD 1 ]---------------+------- +assigned_node_id | 13 +assigned_group_id | 0 +assigned_group_state | single +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | ff_p + +SELECT nodeid AS np FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_p' \gset +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_s1', 5432, + 'postgres', 'ff_s1', 1); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 14 +assigned_group_id | 0 +assigned_group_state | wait_standby +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | ff_s1 + +SELECT nodeid AS ns1 FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_s1' \gset +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_s2', 5432, + 'postgres', 'ff_s2', 1); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 15 +assigned_group_id | 0 +assigned_group_state | wait_standby +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | ff_s2 + +SELECT nodeid AS ns2 FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_s2' \gset +-- ── bootstrap ──────────────────────────────────────────────────────────────── +-- p: single (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'single'); +-[ RECORD 1 ]--------+------- +assigned_group_state | single + +-- s1: wait_standby (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_standby + +-- p: single → wait_primary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'single', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +-- p: wait_primary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +-- s1: wait_standby → catchingup +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +-- s1: catchingup → secondary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +-- s1: secondary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +-- p: wait_primary → primary (s1 is now secondary) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +-- p: primary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +-- s1: secondary (confirm after primary confirmed) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +-- s2: wait_standby (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +-- s2: catchingup +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +-- s2: secondary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +-- p: primary (refresh to pick up second secondary) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+--------------- +assigned_group_state | apply_settings + +-- Verify final formation state: p=primary, s1=secondary, s2=secondary. +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; +-[ RECORD 1 ]-+--------------- +nodename | ff_p +goalstate | apply_settings +reportedstate | primary +-[ RECORD 2 ]-+--------------- +nodename | ff_s1 +goalstate | secondary +reportedstate | secondary +-[ RECORD 3 ]-+--------------- +nodename | ff_s2 +goalstate | catchingup +reportedstate | secondary + +-- ── set up fast_forward scenario ───────────────────────────────────────────── +-- +-- s2 has higher candidate priority so the monitor elects it as fast_forward +-- candidate. s1 has the higher LSN and stays in report_lsn (WAL source). +-- We then make s1 unhealthy so s2's WAL fetch has no valid source. +SET pgautofailover.startup_grace_period = 1; +-- Mark p as dead. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'ff_test' AND nodename = 'ff_p'; +-- Demote p to draining. +UPDATE pgautofailover.node + SET goalstate = 'draining', reportedstate = 'draining' + WHERE formationid = 'ff_test' AND nodename = 'ff_p'; +-- s1 reported a higher LSN (0/5100) in the report_lsn round. +UPDATE pgautofailover.node + SET goalstate = 'report_lsn', + reportedstate = 'report_lsn', + reportedlsn = '0/5100' + WHERE formationid = 'ff_test' AND nodename = 'ff_s1'; +-- s2 has candidate_priority 90 < 100 (lower than s1's default 100) but for +-- the purpose of this test we give it priority 110 so the monitor prefers it. +-- We then place s2 directly in fast_forward/report_lsn (goal assigned by the +-- monitor, not yet confirmed by the keeper). +UPDATE pgautofailover.node + SET candidatepriority = 110, + reportedlsn = '0/5000', + goalstate = 'fast_forward', + reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; +-- Make s1 unhealthy: it cannot serve as a WAL source. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'ff_test' AND nodename = 'ff_s1'; +-- Verify manufactured state before tests. +SELECT nodename, goalstate, reportedstate, reportedlsn, health, candidatepriority + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; +-[ RECORD 1 ]-----+------------- +nodename | ff_p +goalstate | draining +reportedstate | draining +reportedlsn | 0/5000 +health | 0 +candidatepriority | 100 +-[ RECORD 2 ]-----+------------- +nodename | ff_s1 +goalstate | report_lsn +reportedstate | report_lsn +reportedlsn | 0/5100 +health | 0 +candidatepriority | 100 +-[ RECORD 3 ]-----+------------- +nodename | ff_s2 +goalstate | fast_forward +reportedstate | report_lsn +reportedlsn | 0/5000 +health | -1 +candidatepriority | 110 + +-- ── test A: guard_data_loss = true (default) resets goal to report_lsn ────── +-- +-- s2 calls node_active with reportedState=report_lsn while goalState is +-- fast_forward and all WAL source nodes (s1) are unhealthy. +-- The stuck-fast_forward guard must reset s2's goal back to report_lsn. +SET pgautofailover.guard_data_loss TO true; +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'report_lsn', + current_lsn => '0/5000', + current_tli => 1, + current_pg_is_running => true); +-[ RECORD 1 ]--------+----------- +assigned_group_state | report_lsn + +-- s2 goal must now be report_lsn (reset by the guard). +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; +-[ RECORD 1 ]-+----------- +nodename | ff_p +goalstate | draining +reportedstate | draining +-[ RECORD 2 ]-+----------- +nodename | ff_s1 +goalstate | report_lsn +reportedstate | report_lsn +-[ RECORD 3 ]-+----------- +nodename | ff_s2 +goalstate | report_lsn +reportedstate | report_lsn + +-- ── re-arm fast_forward scenario for test B ────────────────────────────────── +-- +-- Put s2 back into fast_forward goal / report_lsn reported state, +-- s1 still unhealthy. +UPDATE pgautofailover.node + SET goalstate = 'fast_forward', reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; +-- Confirm re-armed state. +SELECT nodename, goalstate, reportedstate, health + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; +-[ RECORD 1 ]-+------------- +nodename | ff_p +goalstate | draining +reportedstate | draining +health | 0 +-[ RECORD 2 ]-+------------- +nodename | ff_s1 +goalstate | report_lsn +reportedstate | report_lsn +health | 0 +-[ RECORD 3 ]-+------------- +nodename | ff_s2 +goalstate | fast_forward +reportedstate | report_lsn +health | -1 + +-- ── test B: guard_data_loss = false — goal stays fast_forward ──────────────── +-- +-- The guard logs the situation but does NOT reset s2's goal; it falls through +-- to ProceedWithMSFailover. get_most_advanced_standby() will find no healthy +-- WAL source and fsm_fast_forward() will skip the fetch, causing the keeper to +-- call node_active again with reportedState=fast_forward, at which point the +-- monitor assigns prepare_promotion. Here we only verify that the immediate +-- node_active response does NOT return report_lsn (goal is kept or advanced). +SET pgautofailover.guard_data_loss TO false; +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'report_lsn', + current_lsn => '0/5000', + current_tli => 1, + current_pg_is_running => true); +-[ RECORD 1 ]--------+------------- +assigned_group_state | fast_forward + +-- s2 goal must NOT be reset to report_lsn; it should be fast_forward or +-- further advanced (prepare_promotion). +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; +-[ RECORD 1 ]-+------------- +nodename | ff_p +goalstate | draining +reportedstate | draining +-[ RECORD 2 ]-+------------- +nodename | ff_s1 +goalstate | report_lsn +reportedstate | report_lsn +-[ RECORD 3 ]-+------------- +nodename | ff_s2 +goalstate | fast_forward +reportedstate | report_lsn + +-- ── test get_most_advanced_standby directly ─────────────────────────────────── +-- +-- Restore s2 to fast_forward / report_lsn so both nodes are in report_lsn. +UPDATE pgautofailover.node + SET goalstate = 'fast_forward', reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; +-- guard_data_loss=true: s1 is included despite health=0 (SQL filter uses OR). +-- Caller is s2 (:ns2) so s1 should be returned (highest LSN among others). +SET pgautofailover.guard_data_loss TO true; +SELECT node_name, node_lsn, node_is_primary + FROM pgautofailover.get_most_advanced_standby('ff_test', 0, :ns2); +-[ RECORD 1 ]---+------- +node_name | ff_s1 +node_lsn | 0/5100 +node_is_primary | f + +-- guard_data_loss=false: unhealthy nodes are excluded; s1 (health=0) is +-- filtered out. s2 itself is excluded via caller_node_id. Expect no rows. +SET pgautofailover.guard_data_loss TO false; +SELECT node_name, node_lsn, node_is_primary + FROM pgautofailover.get_most_advanced_standby('ff_test', 0, :ns2); +(0 rows) + +-- ── cleanup ─────────────────────────────────────────────────────────────────── +RESET pgautofailover.guard_data_loss; +RESET pgautofailover.startup_grace_period; diff --git a/src/monitor/group_state_machine.c b/src/monitor/group_state_machine.c index 0ad622dfa..e9cd94247 100644 --- a/src/monitor/group_state_machine.c +++ b/src/monitor/group_state_machine.c @@ -1292,6 +1292,46 @@ ProceedGroupStateForPrimaryNode(GroupStateContext *ctx, } +/* + * WalSourceNodesAreAllUnhealthy returns true when every report_lsn peer that + * could serve as a WAL source for the given fast_forward candidate is + * currently unhealthy. Returns false if at least one source is healthy, or + * if there are no source nodes at all (unusual; caller handles separately). + */ +static bool +WalSourceNodesAreAllUnhealthy(GroupStateContext *ctx, + List *nodesGroupList, + AutoFailoverNode *candidateNode) +{ + ListCell *nodeCell = NULL; + bool foundAnySource = false; + + foreach(nodeCell, nodesGroupList) + { + AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); + + if (node->nodeId == candidateNode->nodeId) + { + continue; + } + + if (!IsCurrentState(node, REPLICATION_STATE_REPORT_LSN)) + { + continue; + } + + foundAnySource = true; + + if (NodeIsHealthy(node, ctx)) + { + return false; + } + } + + return foundAnySource; +} + + /* * ProceedGroupStateForMSFailover implements Group State Machine transition to * orchestrate a failover when we have more than one standby. @@ -1335,6 +1375,59 @@ ProceedGroupStateForMSFailover(GroupStateContext *ctx, /* activeNode might be the failover candidate, proceed already */ if (nodeBeingPromoted->nodeId == activeNode->nodeId) { + /* + * Detect a fast_forward candidate whose WAL fetch failed: the + * keeper reports back report_lsn while the goal is still + * fast_forward. + * + * When all WAL source nodes (other report_lsn peers) are + * unhealthy we cannot make progress without data loss. Warn the + * operator and act based on guard_data_loss: + * + * - guard_data_loss=true: reset the candidate goal back to + * report_lsn so the cycle retries automatically if a source + * recovers. + * + * - guard_data_loss=false: log and fall through. + * get_most_advanced_standby() filters unhealthy sources out, + * so fsm_fast_forward() will find no upstream, skip the WAL + * fetch, and report fast_forward as its current state. The + * monitor then assigns prepare_promotion on the next call. + */ + if (activeNode->reportedState == REPLICATION_STATE_REPORT_LSN && + activeNode->goalState == REPLICATION_STATE_FAST_FORWARD && + WalSourceNodesAreAllUnhealthy(ctx, nodesGroupList, activeNode)) + { + if (GuardDataLoss) + { + LogAndNotifyMessage( + message, BUFSIZE, + "Failover candidate " NODE_FORMAT + " is stuck in fast_forward: all WAL source nodes are " + "unhealthy and pgautofailover.guard_data_loss is true. " + "Resetting candidate to report_lsn to retry when a " + "source recovers. Use pg_autoctl perform failover " + "--allow-data-loss to promote with available WAL.", + NODE_FORMAT_ARGS(activeNode)); + + AssignGoalState(activeNode, + REPLICATION_STATE_REPORT_LSN, + message); + + return true; + } + else + { + LogAndNotifyMessage( + message, BUFSIZE, + "Failover candidate " NODE_FORMAT + " is in fast_forward with all WAL source nodes " + "unhealthy; pgautofailover.guard_data_loss is false, " + "will promote with available WAL.", + NODE_FORMAT_ARGS(activeNode)); + } + } + return ProceedWithMSFailover(activeNode, nodeBeingPromoted); } diff --git a/src/monitor/pgautofailover.sql b/src/monitor/pgautofailover.sql index 08e19033a..fcf120725 100644 --- a/src/monitor/pgautofailover.sql +++ b/src/monitor/pgautofailover.sql @@ -401,6 +401,7 @@ CREATE FUNCTION pgautofailover.get_most_advanced_standby ( IN formationid text default 'default', IN groupid int default 0, + IN caller_node_id bigint default 0, OUT node_id bigint, OUT node_name text, OUT node_host text, @@ -414,12 +415,14 @@ AS $$ from pgautofailover.node where formationid = $1 and groupid = $2 + and nodeid != $3 and reportedstate = 'report_lsn' + and (current_setting('pgautofailover.guard_data_loss')::bool or health > 0) order by reportedlsn desc, health desc limit 1; $$; -grant execute on function pgautofailover.get_most_advanced_standby(text,int) +grant execute on function pgautofailover.get_most_advanced_standby(text,int,bigint) to autoctl_node; diff --git a/src/monitor/sql/fast_forward.sql b/src/monitor/sql/fast_forward.sql new file mode 100644 index 000000000..cc3bda9c9 --- /dev/null +++ b/src/monitor/sql/fast_forward.sql @@ -0,0 +1,280 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression tests for the fast_forward WAL-fetch stuck detection introduced +-- in group_state_machine.c (ProceedGroupStateForMSFailover). +-- +-- Scenario: 3-node formation (p + s1 + s2, number_sync_standbys=1). +-- p dies → draining. +-- s1 has the higher LSN (0/5100) → stays in report_lsn (WAL source). +-- s2 has a lower LSN (0/5000) but higher candidate_priority → selected as +-- fast_forward candidate. +-- s1 then becomes unhealthy → s2's WAL fetch has no source. +-- +-- Test A (guard_data_loss=true): +-- node_active for s2 with reportedState=report_lsn resets s2 goal to +-- report_lsn (stuck-fast_forward guard fires). +-- +-- Test B (guard_data_loss=false): +-- node_active for s2 with reportedState=report_lsn falls through; s2 goal +-- stays fast_forward. +-- +-- Also exercises get_most_advanced_standby() directly: +-- guard_data_loss=true → returns s1 (unhealthy but included by SQL filter). +-- guard_data_loss=false → returns no rows (unhealthy nodes filtered out, +-- s2 excluded by caller_node_id). + +\x on + +-- ── formation setup ────────────────────────────────────────────────────────── + +SELECT pgautofailover.create_formation('ff_test', 'pgsql', 'postgres', true, 1); + +-- Register three nodes. +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_p', 5432, + 'postgres', 'ff_p', 1); + +SELECT nodeid AS np FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_p' \gset + +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_s1', 5432, + 'postgres', 'ff_s1', 1); + +SELECT nodeid AS ns1 FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_s1' \gset + +SELECT * + FROM pgautofailover.register_node('ff_test', 'ff_s2', 5432, + 'postgres', 'ff_s2', 1); + +SELECT nodeid AS ns2 FROM pgautofailover.node + WHERE formationid = 'ff_test' AND nodename = 'ff_s2' \gset + +-- ── bootstrap ──────────────────────────────────────────────────────────────── + +-- p: single (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'single'); + +-- s1: wait_standby (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'wait_standby'); + +-- p: single → wait_primary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'single', + current_lsn => '0/5000'); + +-- p: wait_primary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +-- s1: wait_standby → catchingup +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'wait_standby'); + +-- s1: catchingup → secondary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +-- s1: secondary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +-- p: wait_primary → primary (s1 is now secondary) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +-- p: primary (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); + +-- s1: secondary (confirm after primary confirmed) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +-- s2: wait_standby (confirm) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'wait_standby'); + +-- s2: catchingup +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +-- s2: secondary +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +-- p: primary (refresh to pick up second secondary) +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); + +-- Verify final formation state: p=primary, s1=secondary, s2=secondary. +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; + +-- ── set up fast_forward scenario ───────────────────────────────────────────── +-- +-- s2 has higher candidate priority so the monitor elects it as fast_forward +-- candidate. s1 has the higher LSN and stays in report_lsn (WAL source). +-- We then make s1 unhealthy so s2's WAL fetch has no valid source. + +SET pgautofailover.startup_grace_period = 1; + +-- Mark p as dead. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'ff_test' AND nodename = 'ff_p'; + +-- Demote p to draining. +UPDATE pgautofailover.node + SET goalstate = 'draining', reportedstate = 'draining' + WHERE formationid = 'ff_test' AND nodename = 'ff_p'; + +-- s1 reported a higher LSN (0/5100) in the report_lsn round. +UPDATE pgautofailover.node + SET goalstate = 'report_lsn', + reportedstate = 'report_lsn', + reportedlsn = '0/5100' + WHERE formationid = 'ff_test' AND nodename = 'ff_s1'; + +-- s2 has candidate_priority 90 < 100 (lower than s1's default 100) but for +-- the purpose of this test we give it priority 110 so the monitor prefers it. +-- We then place s2 directly in fast_forward/report_lsn (goal assigned by the +-- monitor, not yet confirmed by the keeper). +UPDATE pgautofailover.node + SET candidatepriority = 110, + reportedlsn = '0/5000', + goalstate = 'fast_forward', + reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; + +-- Make s1 unhealthy: it cannot serve as a WAL source. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'ff_test' AND nodename = 'ff_s1'; + +-- Verify manufactured state before tests. +SELECT nodename, goalstate, reportedstate, reportedlsn, health, candidatepriority + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; + +-- ── test A: guard_data_loss = true (default) resets goal to report_lsn ────── +-- +-- s2 calls node_active with reportedState=report_lsn while goalState is +-- fast_forward and all WAL source nodes (s1) are unhealthy. +-- The stuck-fast_forward guard must reset s2's goal back to report_lsn. + +SET pgautofailover.guard_data_loss TO true; + +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'report_lsn', + current_lsn => '0/5000', + current_tli => 1, + current_pg_is_running => true); + +-- s2 goal must now be report_lsn (reset by the guard). +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; + +-- ── re-arm fast_forward scenario for test B ────────────────────────────────── +-- +-- Put s2 back into fast_forward goal / report_lsn reported state, +-- s1 still unhealthy. + +UPDATE pgautofailover.node + SET goalstate = 'fast_forward', reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; + +-- Confirm re-armed state. +SELECT nodename, goalstate, reportedstate, health + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; + +-- ── test B: guard_data_loss = false — goal stays fast_forward ──────────────── +-- +-- The guard logs the situation but does NOT reset s2's goal; it falls through +-- to ProceedWithMSFailover. get_most_advanced_standby() will find no healthy +-- WAL source and fsm_fast_forward() will skip the fetch, causing the keeper to +-- call node_active again with reportedState=fast_forward, at which point the +-- monitor assigns prepare_promotion. Here we only verify that the immediate +-- node_active response does NOT return report_lsn (goal is kept or advanced). + +SET pgautofailover.guard_data_loss TO false; + +SELECT assigned_group_state + FROM pgautofailover.node_active('ff_test', :ns2, 0, + current_group_role => 'report_lsn', + current_lsn => '0/5000', + current_tli => 1, + current_pg_is_running => true); + +-- s2 goal must NOT be reset to report_lsn; it should be fast_forward or +-- further advanced (prepare_promotion). +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'ff_test' + ORDER BY nodename; + +-- ── test get_most_advanced_standby directly ─────────────────────────────────── +-- +-- Restore s2 to fast_forward / report_lsn so both nodes are in report_lsn. + +UPDATE pgautofailover.node + SET goalstate = 'fast_forward', reportedstate = 'report_lsn' + WHERE formationid = 'ff_test' AND nodename = 'ff_s2'; + +-- guard_data_loss=true: s1 is included despite health=0 (SQL filter uses OR). +-- Caller is s2 (:ns2) so s1 should be returned (highest LSN among others). +SET pgautofailover.guard_data_loss TO true; + +SELECT node_name, node_lsn, node_is_primary + FROM pgautofailover.get_most_advanced_standby('ff_test', 0, :ns2); + +-- guard_data_loss=false: unhealthy nodes are excluded; s1 (health=0) is +-- filtered out. s2 itself is excluded via caller_node_id. Expect no rows. +SET pgautofailover.guard_data_loss TO false; + +SELECT node_name, node_lsn, node_is_primary + FROM pgautofailover.get_most_advanced_standby('ff_test', 0, :ns2); + +-- ── cleanup ─────────────────────────────────────────────────────────────────── + +RESET pgautofailover.guard_data_loss; +RESET pgautofailover.startup_grace_period; diff --git a/tests/tap/schedule b/tests/tap/schedule index 024b4bf81..a90a0ad67 100644 --- a/tests/tap/schedule +++ b/tests/tap/schedule @@ -23,6 +23,7 @@ multi_ifdown multi_maintenance multi_alternate guard_data_loss +fast_forward extension_update installcheck # Upgrade test — requires pgaf:current and pgaf:next images to be pre-built: diff --git a/tests/tap/specs/fast_forward.pgaf b/tests/tap/specs/fast_forward.pgaf new file mode 100644 index 000000000..6b1c405ef --- /dev/null +++ b/tests/tap/specs/fast_forward.pgaf @@ -0,0 +1,129 @@ +# Test fast_forward stuck detection and recovery +# +# Scenario: 3-node formation (number_sync_standbys=1). +# When a failover candidate is assigned the fast_forward state, it means the +# candidate has a lower LSN than another standby (the "WAL source") and must +# fetch WAL from that standby before being promoted. If the WAL source dies +# while the candidate is fetching, the candidate gets stuck in fast_forward +# with no live peer to catch up from. +# +# The fix (issue #1060) adds stuck-detection logic to the monitor: +# - guard_data_loss=true → monitor resets the candidate back to report_lsn +# so that the next health-check cycle can re-evaluate the situation. +# - guard_data_loss=false → monitor allows the candidate to promote with +# whatever WAL it already has. +# +# Because a tap test cannot easily pre-load one standby with more WAL than +# another, this test exercises the guard_data_loss=false end-to-end path: +# • node1 (primary) and node3 (the WAL-source standby) are killed together. +# • node2 is the only survivor. With guard_data_loss=true the SQL health +# filter sees a missing quorum member and keeps node2 at report_lsn. +# • After running `pg_autoctl perform failover --allow-data-loss`, the +# guard_data_loss GUC is set to false for that transaction and the monitor +# promotes node2 even though its WAL source (node3) is unreachable. +# • node3 and node1 are brought back; the cluster stabilises at +# primary + secondary + secondary. +# +# Related issues: #1060, #1113 + +cluster { + monitor + ssl off + formation { + node1 + node2 + node3 + } +} + +setup { + wait until primary, secondary timeout 120s + wait until node3 state is secondary timeout 60s + promote node1 + wait until node1 state is primary + and node2 state is secondary + and node3 state is secondary + timeout 120s +} + +teardown { + compose down +} + +# +# Step 1: kill the primary (node1) and the WAL-source standby (node3) +# simultaneously. +# +# After node1 dies the monitor assigns report_lsn to both standbys. +# node2 reports its LSN; node3 is killed before it can do so and remains a +# "missing" quorum member. With guard_data_loss=true (the default), the +# monitor's ProceedGroupStateForMSFailover sees missingNodesCount=1 and +# refuses to promote node2. If the monitor had already assigned fast_forward +# to node2 (pointing to node3 as the WAL source) before node3 was detected +# dead, the stuck-detection logic resets node2 back to report_lsn. +# + +step test_001_kill_primary_and_wal_source { + compose kill node1 + compose kill node3 + wait until node1 assigned-state = draining timeout 120s + wait until node2 state is report_lsn timeout 120s +} + +# +# Step 2: verify node2 stays stuck — no automatic promotion. +# +# We sleep briefly to let the monitor run a few more health-check cycles. +# If the stuck-detection fix were absent (or guard_data_loss incorrectly +# false), node2 would have advanced to prepare_promotion by now. +# + +step test_002_verify_stuck { + sleep 10s + assert node2 state is report_lsn +} + +# +# Step 3: unblock with --allow-data-loss. +# +# pg_autoctl perform failover --allow-data-loss opens a transaction on the +# monitor, sets LOCAL pgautofailover.guard_data_loss = false, and calls +# perform_failover(). The health filter no longer blocks on the missing WAL +# source; node2 is selected as the only available candidate and driven toward +# prepare_promotion then wait_primary. +# + +step test_003_allow_data_loss_failover { + exec monitor pg_autoctl perform failover --allow-data-loss --formation default + wait until node2 state is wait_primary timeout 120s +} + +# +# Step 4: bring node3 back so node2 can become a full primary. +# + +step test_004_bringup_node3 { + compose start node3 + wait until node2 state is primary + and node3 state is secondary + timeout 180s +} + +# +# Step 5: bring node1 back as the second standby. +# + +step test_005_bringup_node1 { + compose start node1 + wait until node1 state is secondary + and node2 state is primary + and node3 state is secondary + timeout 120s +} + +sequence + test_001_kill_primary_and_wal_source + test_002_verify_stuck + test_003_allow_data_loss_failover + test_004_bringup_node3 + test_005_bringup_node1