diff --git a/Makefile b/Makefile index 47f05753e..9f934af4a 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,26 @@ check: check-monitor ; check-monitor: install-monitor $(MAKE) -C src/monitor/ installcheck +# Run SQL regression tests (pg_regress installcheck) inside a Docker container. +# Defaults to PG16; override with PGVERSION=17 etc. +# Usage: make installcheck [PGVERSION=16] +INSTALLCHECK_PGVERSION ?= 16 +.PHONY: installcheck +installcheck: build-test-pg$(INSTALLCHECK_PGVERSION) + docker run --rm \ + $(TEST_CONTAINER_NAME):pg$(INSTALLCHECK_PGVERSION) \ + bash -c ' \ + initdb --auth=trust --username=docker -D /tmp/pgdata && \ + printf "shared_preload_libraries = '"'"'pgautofailover'"'"'\n" \ + >> /tmp/pgdata/postgresql.conf && \ + pg_ctl start -D /tmp/pgdata -l /tmp/pgdata/pg.log -o "-k /tmp -p 5432" && \ + sudo chmod -R a+w /usr/src/pg_auto_failover/src/monitor && \ + make -C /usr/src/pg_auto_failover/src/monitor installcheck \ + PGHOST=/tmp PGPORT=5432 PGUSER=docker \ + || { pg_ctl stop -D /tmp/pgdata; exit 1; } && \ + pg_ctl stop -D /tmp/pgdata \ + ' + .PHONY: clean clean: clean-monitor clean-bin ; diff --git a/src/monitor/Makefile b/src/monitor/Makefile index f5f826d19..39606a2e4 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 dummy_update drop_extension upgrade +REGRESS = create_extension monitor workers node_active_protocol dummy_update drop_extension upgrade PG_CONFIG ?= pg_config PGXS = $(shell $(PG_CONFIG) --pgxs) diff --git a/src/monitor/expected/node_active_protocol.out b/src/monitor/expected/node_active_protocol.out new file mode 100644 index 000000000..203e63f6e --- /dev/null +++ b/src/monitor/expected/node_active_protocol.out @@ -0,0 +1,598 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression tests for the monitor's node_active() protocol. +-- +-- Covers two concrete regressions: +-- +-- #1062 — Primary has health=BAD (health-check worker cannot reach it) but +-- IS calling node_active with pgIsRunning=true. NodeIsHealthy must +-- return true via the fresh-report override so no spurious failover +-- is triggered. +-- +-- #1032 — After a failover the old primary catches up while the health +-- checker still marks it BAD. The fixed NodeIsHealthy() allows +-- catchingup→secondary to proceed. Without the fix the transition +-- was permanently blocked (NodeIsHealthy returned false for any +-- node with health=BAD regardless of the live report). +-- +-- Additional regression: NODE_HEALTH_UNKNOWN (-1) is the initial DB value for +-- newly registered nodes (no health-check worker has run yet). NodeIsHealthy +-- must treat UNKNOWN as "trust the keeper's own pgIsRunning report." This +-- manifests during formation bootstrap: the catchingup→secondary transition +-- requires NodeIsHealthy(joining-node) to be true. +-- +-- Also exercises the stale in-memory struct fix in NodeActive(): after +-- ReportAutoFailoverNodeState writes pgIsRunning to the DB, the same value +-- must be synced back into the pgAutoFailoverNode struct before +-- ProceedGroupState runs, otherwise pgIsRunning appears stale to +-- NodeIsHealthy when the keeper changes from pgIsRunning=false to true. +\x on +-- ── formation and node registration ───────────────────────────────────────── +SELECT pgautofailover.create_formation('fsm_test', 'pgsql', 'postgres', true, 1); +-[ RECORD 1 ]----+------------------------------ +create_formation | (fsm_test,pgsql,postgres,t,1) + +-- sysidentifier=1: a non-zero value satisfies the same_system_identifier +-- constraint without a separate set_node_system_identifier() call. +SELECT * + FROM pgautofailover.register_node('fsm_test', 'node1', 5432, + 'postgres', 'node1', 1); +-[ RECORD 1 ]---------------+------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | single +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | node1 + +SELECT nodeid AS n1 FROM pgautofailover.node + WHERE formationid = 'fsm_test' AND nodename = 'node1' \gset +SELECT * + FROM pgautofailover.register_node('fsm_test', 'node2', 5432, + 'postgres', 'node2', 1); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | wait_standby +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | node2 + +SELECT nodeid AS n2 FROM pgautofailover.node + WHERE formationid = 'fsm_test' AND nodename = 'node2' \gset +-- ── formation bootstrap ────────────────────────────────────────────────────── +-- +-- IsCurrentState(node, S) requires both goalState=S and reportedState=S, so +-- every intermediate state must be explicitly confirmed before the next guard +-- fires. The sequence below mirrors what two real keepers would produce. +-- node1: single (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'single'); +-[ RECORD 1 ]---------------+------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | single +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2: wait_standby (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | wait_standby +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1: single → wait_primary (secondary has joined in WAIT_STANDBY) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'single', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | wait_primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1: wait_primary (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | wait_primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2: wait_standby → catchingup (primary is confirmed in WAIT_PRIMARY) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]---------------+----------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | catchingup +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2: catchingup → secondary +-- NODE_HEALTH_UNKNOWN (-1) regression: health is -1 (default for new nodes; +-- the health-check worker has not run). NodeIsHealthy must trust the +-- keeper's pgIsRunning=true instead of returning false. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2: secondary (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1: wait_primary → primary (secondary quorum satisfied) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- ── test_001: steady state ─────────────────────────────────────────────────── +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- ── test_002: issue #1062 ───────────────────────────────────────────────────── +-- +-- Health checker marks primary BAD while the primary is still calling +-- node_active with pgIsRunning=true. NodeIsHealthy must return true via the +-- fresh-report override: +-- health=BAD, healthchecktime < reportTime, reportTime within 1s of now. +-- No spurious failover must be triggered. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; +-- Primary reports pgIsRunning=true; fresh report overrides the BAD health. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- Secondary calls in; NodeIsUnhealthy(primary) is false — no promotion. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- restore health before the full-failure test +UPDATE pgautofailover.node + SET health = 1, + healthchecktime = now() + WHERE nodename = 'node1'; +-- ── test_003: two-node failover ─────────────────────────────────────────────── +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; +-- Primary's own heartbeat with pgIsRunning=false. In the two-node case +-- ProceedGroupStateForPrimaryNode has no self-demotion rule; the failover +-- trigger fires from the standby's heartbeat. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => false, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- Secondary calls in: NodeIsUnhealthy(primary) is true → prepare_promotion. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+------------------ +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | prepare_promotion +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2 reports prepare_promotion → assigned stop_replication. +-- node1 gets goalState=demote_timeout. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'prepare_promotion', + current_pg_is_running => true, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+----------------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | stop_replication +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1 reports demote_timeout; satisfies IsCurrentState(node1, DEMOTE_TIMEOUT) +-- so that the next node2 heartbeat can advance. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'demote_timeout', + current_pg_is_running => false, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+--------------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | demote_timeout +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2 reports stop_replication → assigned wait_primary. +-- node1 gets goalState=demoted. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'stop_replication', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | wait_primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2 in wait_primary; no secondary in quorum yet. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | wait_primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- ── test_004: issue #1032 ───────────────────────────────────────────────────── +-- +-- Old primary (node1) resurfaces. Health checker is still marking it BAD +-- (recovery in progress). The fixed NodeIsHealthy() must allow the +-- catchingup→secondary transition. +-- +-- The stale in-memory struct fix is also exercised here: node1's previous +-- node_active call reported pgIsRunning=false (demoted). The DB therefore +-- has reportedpgisrunning=false. When node1 reports catchingup with +-- pgIsRunning=true, ReportAutoFailoverNodeState writes true to the DB but the +-- in-memory struct still holds false. Without the fix, ProceedGroupState +-- sees pgIsRunning=false and NodeIsHealthy returns false, permanently blocking +-- the transition. +UPDATE pgautofailover.node + SET health = 1, + healthchecktime = now() + WHERE nodename = 'node1'; +-- node1 resurfaces reporting 'primary' while its goalState is 'demoted'. +-- IsCurrentState(node1, DEMOTED) is false (reportedState mismatch), so no +-- DEMOTED rule fires; the monitor returns the current goalState = demoted. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_tli => 1, + current_lsn => '0/4F00'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | demoted +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1 reports demoted → catchingup assigned. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'demoted', + current_pg_is_running => false, + current_tli => 1, + current_lsn => '0/4F00'); +-[ RECORD 1 ]---------------+----------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | catchingup +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- Health checker marks node1 BAD again (recovery still in progress). +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; +-- node1 calls node_active with pgIsRunning=true. +-- NodeIsHealthy(node1) returns true via the fresh-report override. +-- The stale-struct fix ensures the pgIsRunning=true from this call's report +-- is visible to ProceedGroupState within the same call. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'catchingup', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node1 confirms secondary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- node2 now has a healthy secondary in the quorum → promoted to primary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- ── test_005: start_maintenance on primary with health=UNKNOWN secondary ────── +-- +-- Regression for the CountHealthyCandidates / IsHealthy inconsistency. +-- +-- NodeIsHealthy() treats NODE_HEALTH_UNKNOWN as "trust pgIsRunning", so +-- catchingup→secondary can fire before the health-check worker runs. The +-- old IsHealthy() returned false for UNKNOWN, so start_maintenance() counted +-- 0 healthy candidates even when the secondary was fully reachable. +-- +-- After fixing IsHealthy() to mirror NodeIsHealthy() for UNKNOWN health, +-- start_maintenance must succeed and assign prepare_maintenance / +-- prepare_promotion. +-- Confirm node2 as primary (last test_004 call left it in wait_primary goal). +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+-------- +assigned_node_id | 7 +assigned_group_id | 0 +assigned_group_state | primary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- Confirm node1 as secondary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); +-[ RECORD 1 ]---------------+---------- +assigned_node_id | 6 +assigned_group_id | 0 +assigned_group_state | secondary +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +-- Force node1 health back to UNKNOWN to simulate a node that reached +-- SECONDARY before the health-check worker ran (the window our FSM fix +-- opened up). Without the IsHealthy() fix this causes start_maintenance +-- to fail with "0 candidate nodes available". +UPDATE pgautofailover.node + SET health = -1 + WHERE nodename = 'node1'; +-- start_maintenance on the primary (node2): must succeed despite +-- node1 health=UNKNOWN, because IsHealthy() now trusts pgIsRunning=true +-- when no health check has run yet. +SELECT pgautofailover.start_maintenance(:n2); +-[ RECORD 1 ]-----+-- +start_maintenance | t + +-- Verify: node2 → prepare_maintenance, node1 → prepare_promotion. +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'fsm_test' + ORDER BY nodename; +-[ RECORD 1 ]-+-------------------- +nodename | node1 +goalstate | prepare_promotion +reportedstate | secondary +-[ RECORD 2 ]-+-------------------- +nodename | node2 +goalstate | prepare_maintenance +reportedstate | primary + +-- ── test_006: killed-primary failover (health=BAD, pgIsRunning still true) ──── +-- +-- The container-killed scenario: the primary stops reporting node_active (so +-- pgIsRunning stays TRUE in the DB from its last heartbeat), the health-check +-- worker eventually marks it BAD, and after unhealthyTimeoutMs the monitor +-- should fire secondary → prepare_promotion even though the primary never +-- self-reported pgIsRunning=false. +-- +-- Distinct from test_003 (self-reported-down path via pgIsRunning=false). +-- Here we exercise the time+health-check-based unhealthy path. +-- +-- Uses a fresh formation to start from a clean state. +SELECT pgautofailover.create_formation('killed_test', 'pgsql', 'postgres', true, 0); +-[ RECORD 1 ]----+--------------------------------- +create_formation | (killed_test,pgsql,postgres,t,0) + +SELECT * + FROM pgautofailover.register_node('killed_test', 'ka', 5432, + 'postgres', 'ka', 1); +-[ RECORD 1 ]---------------+------- +assigned_node_id | 8 +assigned_group_id | 0 +assigned_group_state | single +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | ka + +SELECT nodeid AS ka FROM pgautofailover.node + WHERE formationid = 'killed_test' AND nodename = 'ka' \gset +SELECT * + FROM pgautofailover.register_node('killed_test', 'kb', 5432, + 'postgres', 'kb', 1); +-[ RECORD 1 ]---------------+------------- +assigned_node_id | 9 +assigned_group_id | 0 +assigned_group_state | wait_standby +assigned_candidate_priority | 100 +assigned_replication_quorum | t +assigned_node_name | kb + +SELECT nodeid AS kb FROM pgautofailover.node + WHERE formationid = 'killed_test' AND nodename = 'kb' \gset +-- Bootstrap to primary + secondary. +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'single'); +-[ RECORD 1 ]--------+------- +assigned_group_state | single + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_standby + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'single', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'wait_primary', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'catchingup', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'secondary', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'wait_primary', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'primary', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'secondary', current_lsn => '0/3000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +-- Lower startup_grace_period to 1ms so the time-based unhealthy path fires +-- immediately in the test environment (in production the server has been up +-- far longer than the 10s default). +SET pgautofailover.startup_grace_period = 1; +-- Simulate killed primary (ka): health checker ran and marked it BAD, +-- but ka's last node_active set pgIsRunning=true and that value is still in +-- the DB — it never self-reported pgIsRunning=false. +-- reporttime is set 60s in the past to exceed UnhealthyTimeoutMs (20s default). +-- healthchecktime is recent so the health-check-ran-after-startup condition holds. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'killed_test' AND nodename = 'ka'; +-- kb (secondary) calls node_active: NodeIsUnhealthy(ka, ctx) must fire via +-- the time-based path (reportTime > unhealthyTimeoutMs AND health=BAD), +-- triggering secondary → prepare_promotion even though ka never called +-- node_active with pgIsRunning=false. +SELECT * + FROM pgautofailover.node_active('killed_test', :kb, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/3000'); +-[ RECORD 1 ]---------------+------------------ +assigned_node_id | 9 +assigned_group_id | 0 +assigned_group_state | prepare_promotion +assigned_candidate_priority | 100 +assigned_replication_quorum | t + +RESET pgautofailover.startup_grace_period; diff --git a/src/monitor/group_state_machine.c b/src/monitor/group_state_machine.c index cf2b3e4e4..145ad6ba6 100644 --- a/src/monitor/group_state_machine.c +++ b/src/monitor/group_state_machine.c @@ -52,16 +52,19 @@ typedef struct CandidateList /* private function forward declarations */ -static bool ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode); -static bool ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, +static bool ProceedGroupStateForPrimaryNode(GroupStateContext *ctx, + AutoFailoverNode *primaryNode); +static bool ProceedGroupStateForMSFailover(GroupStateContext *ctx, AutoFailoverNode *primaryNode); static bool ProceedWithMSFailover(AutoFailoverNode *activeNode, AutoFailoverNode *candidateNode); -static bool BuildCandidateList(List *standbyNodesGroupList, +static bool BuildCandidateList(GroupStateContext *ctx, + List *standbyNodesGroupList, CandidateList *candidateList); -static AutoFailoverNode * SelectFailoverCandidateNode(CandidateList *candidateList, +static AutoFailoverNode * SelectFailoverCandidateNode(GroupStateContext *ctx, + CandidateList *candidateList, AutoFailoverNode *primaryNode); static bool PromoteSelectedNode(AutoFailoverNode *selectedNode, @@ -80,27 +83,72 @@ int PromoteXlogThreshold = DEFAULT_XLOG_SEG_SIZE; /* - * ProceedGroupState proceeds the state machines of the group of which - * the given node is part. + * BuildGroupStateContext loads everything from the database that the + * node_active FSM needs, capturing a single timestamp snapshot and copying + * the current GUC values. Call this once at the top of NodeActive() and pass + * the resulting context to ProceedGroupStateFromContext(). + * + * Returns false (and raises an ereport ERROR) when the formation cannot be + * found. */ bool -ProceedGroupState(AutoFailoverNode *activeNode) +BuildGroupStateContext(GroupStateContext *ctx, AutoFailoverNode *activeNode) { - char *formationId = activeNode->formationId; - int groupId = activeNode->groupId; - - AutoFailoverFormation *formation = GetFormation(formationId); - - List *nodesGroupList = AutoFailoverNodeGroup(formationId, groupId); - int nodesCount = list_length(nodesGroupList); + ctx->formationId = activeNode->formationId; + ctx->groupId = activeNode->groupId; + ctx->activeNode = activeNode; + ctx->formation = GetFormation(activeNode->formationId); + ctx->groupNodeList = + AutoFailoverNodeGroup(activeNode->formationId, activeNode->groupId); + ctx->groupNodeCount = list_length(ctx->groupNodeList); + ctx->now = GetCurrentTimestamp(); + ctx->unhealthyTimeoutMs = UnhealthyTimeoutMs; + ctx->drainTimeoutMs = DrainTimeoutMs; + ctx->startupGracePeriodMs = StartupGracePeriodMs; - if (formation == NULL) + if (ctx->formation == NULL) { ereport(ERROR, (errmsg("Formation for %s could not be found", activeNode->formationId))); } + return true; +} + + +/* + * ProceedGroupState proceeds the state machines of the group of which + * the given node is part. It builds a GroupStateContext from the database and + * delegates to ProceedGroupStateFromContext. + */ +bool +ProceedGroupState(AutoFailoverNode *activeNode) +{ + GroupStateContext ctx; + + BuildGroupStateContext(&ctx, activeNode); + + return ProceedGroupStateFromContext(&ctx); +} + + +/* + * ProceedGroupStateFromContext is the core FSM logic, operating entirely on + * the pre-built GroupStateContext. It does not touch the database for reads; + * writes (AssignGoalState, NotifyStateChange) still go to the DB. + * + * This separation lets test code inject a synthetic context and exercise the + * FSM without a live database connection. + */ +bool +ProceedGroupStateFromContext(GroupStateContext *ctx) +{ + AutoFailoverNode *activeNode = ctx->activeNode; + char *formationId = ctx->formationId; + int groupId = ctx->groupId; + int nodesCount = ctx->groupNodeCount; + /* * If the active node just reached the DROPPED state, proceed to remove it * from the pgautofailover.node table. @@ -191,7 +239,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsInPrimaryState(activeNode)) { - return ProceedGroupStateForPrimaryNode(activeNode); + return ProceedGroupStateForPrimaryNode(ctx, activeNode); } AutoFailoverNode *primaryNode = @@ -214,7 +262,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) * * In all other cases we require a primaryNode to be identified. */ - if (primaryNode == NULL && !IsFailoverInProgress(nodesGroupList)) + if (primaryNode == NULL && !IsFailoverInProgress(ctx->groupNodeList)) { ereport(ERROR, (errmsg("ProceedGroupState couldn't find the primary node " @@ -227,7 +275,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) } /* Multiple Standby failover is handled in its own function. */ - if (nodesCount > 2 && IsUnhealthy(primaryNode)) + if (nodesCount > 2 && NodeIsUnhealthy(primaryNode, ctx)) { /* * The WAIT_PRIMARY state encodes the fact that we know there is no @@ -293,7 +341,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) * stop here. When it return false, it did nothing, and so we want to * apply the common orchestration code for a failover. */ - if (ProceedGroupStateForMSFailover(activeNode, primaryNode)) + if (ProceedGroupStateForMSFailover(ctx, primaryNode)) { return true; } @@ -310,7 +358,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) if (IsCurrentState(activeNode, REPLICATION_STATE_REPORT_LSN) && (IsCurrentState(primaryNode, REPLICATION_STATE_WAIT_PRIMARY) || IsCurrentState(primaryNode, REPLICATION_STATE_JOIN_PRIMARY)) && - IsHealthy(primaryNode)) + NodeIsHealthy(primaryNode, ctx)) { char message[BUFSIZE] = { 0 }; @@ -335,7 +383,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_REPORT_LSN) && IsCurrentState(primaryNode, REPLICATION_STATE_PRIMARY) && - IsHealthy(primaryNode)) + NodeIsHealthy(primaryNode, ctx)) { char message[BUFSIZE]; @@ -378,7 +426,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) if (IsCurrentState(activeNode, REPLICATION_STATE_REPORT_LSN) || IsCurrentState(activeNode, REPLICATION_STATE_FAST_FORWARD)) { - return ProceedGroupStateForMSFailover(activeNode, primaryNode); + return ProceedGroupStateForMSFailover(ctx, primaryNode); } /* @@ -473,7 +521,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) (IsCurrentState(primaryNode, REPLICATION_STATE_WAIT_PRIMARY) || IsCurrentState(primaryNode, REPLICATION_STATE_JOIN_PRIMARY) || IsCurrentState(primaryNode, REPLICATION_STATE_PRIMARY)) && - IsHealthy(activeNode) && + NodeIsHealthy(activeNode, ctx) && activeNode->reportedTLI == primaryNode->reportedTLI && WalDifferenceWithin(activeNode, primaryNode, EnableSyncXlogThreshold)) { @@ -498,7 +546,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_SECONDARY) && IsInPrimaryState(primaryNode) && - IsUnhealthy(primaryNode) && IsHealthy(activeNode) && + NodeIsUnhealthy(primaryNode, ctx) && NodeIsHealthy(activeNode, ctx) && activeNode->candidatePriority > 0 && WalDifferenceWithin(activeNode, primaryNode, PromoteXlogThreshold)) { @@ -603,7 +651,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_PREPARE_PROMOTION) && primaryNode && - IsCitusFormation(formation) && activeNode->groupId > 0) + IsCitusFormation(ctx->formation) && activeNode->groupId > 0) { char message[BUFSIZE]; @@ -630,7 +678,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_PREPARE_PROMOTION) && primaryNode == NULL && - IsCitusFormation(formation) && activeNode->groupId > 0) + IsCitusFormation(ctx->formation) && activeNode->groupId > 0) { char message[BUFSIZE]; @@ -735,7 +783,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_STOP_REPLICATION) && (IsCurrentState(primaryNode, REPLICATION_STATE_DEMOTE_TIMEOUT) || - IsDrainTimeExpired(primaryNode))) + NodeIsDrainTimeExpired(primaryNode, ctx))) { char message[BUFSIZE]; @@ -762,7 +810,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_STOP_REPLICATION) && primaryNode && - IsCitusFormation(formation) && activeNode->groupId > 0) + IsCitusFormation(ctx->formation) && activeNode->groupId > 0) { char message[BUFSIZE]; @@ -789,7 +837,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) */ if (IsCurrentState(activeNode, REPLICATION_STATE_STOP_REPLICATION) && primaryNode == NULL && - IsCitusFormation(formation) && activeNode->groupId > 0) + IsCitusFormation(ctx->formation) && activeNode->groupId > 0) { char message[BUFSIZE]; @@ -815,7 +863,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) * concurrently making progress. */ if (IsCurrentState(activeNode, REPLICATION_STATE_DEMOTED) && - IsHealthy(primaryNode) && + NodeIsHealthy(primaryNode, ctx) && ((primaryNode->reportedState == REPLICATION_STATE_WAIT_PRIMARY || primaryNode->reportedState == REPLICATION_STATE_JOIN_PRIMARY) && primaryNode->goalState == REPLICATION_STATE_PRIMARY)) @@ -841,7 +889,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) * demoted -> catchingup */ if (IsCurrentState(activeNode, REPLICATION_STATE_DEMOTED) && - IsHealthy(primaryNode) && + NodeIsHealthy(primaryNode, ctx) && (IsCurrentState(primaryNode, REPLICATION_STATE_JOIN_PRIMARY) || IsCurrentState(primaryNode, REPLICATION_STATE_WAIT_PRIMARY) || IsCurrentState(primaryNode, REPLICATION_STATE_PRIMARY))) @@ -898,7 +946,7 @@ ProceedGroupState(AutoFailoverNode *activeNode) AssignGoalState(activeNode, REPLICATION_STATE_SECONDARY, message); /* compute next step for the primary depending on node settings */ - return ProceedGroupStateForPrimaryNode(primaryNode); + return ProceedGroupStateForPrimaryNode(ctx, primaryNode); } /* @@ -935,7 +983,8 @@ ProceedGroupState(AutoFailoverNode *activeNode) * Group State Machine when a primary node contacts the monitor. */ static bool -ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) +ProceedGroupStateForPrimaryNode(GroupStateContext *ctx, + AutoFailoverNode *primaryNode) { List *otherNodesGroupList = AutoFailoverOtherNodesList(primaryNode); int otherNodesCount = list_length(otherNodesGroupList); @@ -1011,8 +1060,6 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) int secondaryNodesCount = otherNodesCount; int secondaryQuorumNodesCount = otherNodesCount; - AutoFailoverFormation *formation = - GetFormation(primaryNode->formationId); ListCell *nodeCell = NULL; foreach(nodeCell, otherNodesGroupList) @@ -1029,7 +1076,7 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) if (otherNode->goalState == REPLICATION_STATE_SECONDARY && otherNode->reportedState != REPLICATION_STATE_REPORT_LSN && otherNode->reportedState != REPLICATION_STATE_JOIN_SECONDARY && - IsUnhealthy(otherNode)) + NodeIsUnhealthy(otherNode, ctx)) { char message[BUFSIZE]; @@ -1078,7 +1125,7 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) */ if (replicationQuorumCount == 0) { - Assert(formation->number_sync_standbys == 0); + Assert(ctx->formation->number_sync_standbys == 0); ReplicationState primaryGoalState = secondaryNodesCount == 0 @@ -1133,7 +1180,7 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) * block writes on the primary. */ ReplicationState primaryGoalState = - formation->number_sync_standbys == 0 + ctx->formation->number_sync_standbys == 0 ? REPLICATION_STATE_WAIT_PRIMARY : REPLICATION_STATE_PRIMARY; @@ -1199,7 +1246,7 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) char message[BUFSIZE] = { 0 }; ReplicationState primaryGoalState = - formation->number_sync_standbys == 0 && + ctx->formation->number_sync_standbys == 0 && secondaryQuorumNodesCount == 0 ? REPLICATION_STATE_WAIT_PRIMARY : REPLICATION_STATE_PRIMARY; @@ -1255,11 +1302,11 @@ ProceedGroupStateForPrimaryNode(AutoFailoverNode *primaryNode) * - there's more than one standby node registered in the system */ static bool -ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, +ProceedGroupStateForMSFailover(GroupStateContext *ctx, AutoFailoverNode *primaryNode) { - List *nodesGroupList = - AutoFailoverNodeGroup(activeNode->formationId, activeNode->groupId); + AutoFailoverNode *activeNode = ctx->activeNode; + List *nodesGroupList = ctx->groupNodeList; /* already fetched in context */ CandidateList candidateList = { 0 }; /* @@ -1310,7 +1357,7 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, * started yet. */ if (IsStateIn(nodeBeingPromoted->reportedState, knownUnreachableStates) || - IsHealthy(nodeBeingPromoted)) + NodeIsHealthy(nodeBeingPromoted, ctx)) { elog(LOG, "Found candidate " NODE_FORMAT, NODE_FORMAT_ARGS(nodeBeingPromoted)); @@ -1338,12 +1385,9 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, * different candidateNodesGroupList in which every node has reported their * LSN position, allowing progress to be made. */ - char *formationId = activeNode->formationId; - AutoFailoverFormation *formation = GetFormation(formationId); - - candidateList.numberSyncStandbys = formation->number_sync_standbys; + candidateList.numberSyncStandbys = ctx->formation->number_sync_standbys; - BuildCandidateList(nodesGroupList, &candidateList); + BuildCandidateList(ctx, nodesGroupList, &candidateList); /* * Time to select a candidate? @@ -1385,7 +1429,7 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, * WAIT_PRIMARY state with all the writes blocked for lack of standby * nodes. */ - int minCandidates = formation->number_sync_standbys + 1; + int minCandidates = ctx->formation->number_sync_standbys + 1; /* no candidates is a hard pass */ if (candidateList.candidateCount == 0) @@ -1407,8 +1451,8 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, " and reported state \"%s\"", candidateList.quorumCandidateCount, minCandidates, - formation->number_sync_standbys, - formation->formationId, + ctx->formation->number_sync_standbys, + ctx->formation->formationId, NODE_FORMAT_ARGS(activeNode), ReplicationStateGetName(activeNode->reportedState)); @@ -1457,7 +1501,7 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, } AutoFailoverNode *selectedNode = - SelectFailoverCandidateNode(&candidateList, primaryNode); + SelectFailoverCandidateNode(ctx, &candidateList, primaryNode); /* we might not have a selected candidate for failover yet */ if (selectedNode == NULL) @@ -1507,7 +1551,8 @@ ProceedGroupStateForMSFailover(AutoFailoverNode *activeNode, * caller for BuildCandidateList knows to refrain from any decision making. */ static bool -BuildCandidateList(List *nodesGroupList, CandidateList *candidateList) +BuildCandidateList(GroupStateContext *ctx, List *nodesGroupList, + CandidateList *candidateList) { ListCell *nodeCell = NULL; List *candidateNodesGroupList = NIL; @@ -1550,7 +1595,7 @@ BuildCandidateList(List *nodesGroupList, CandidateList *candidateList) * unless the node is unhealthy because Postgres is down, but * pg_autoctl is still reporting. */ - if (IsUnhealthy(node) && !IsReporting(node)) + if (NodeIsUnhealthy(node, ctx) && !NodeIsReporting(node, ctx)) { elog(LOG, "Skipping candidate " NODE_FORMAT ", which is unhealthy", @@ -1717,7 +1762,8 @@ ProceedWithMSFailover(AutoFailoverNode *activeNode, * the next step (cascade WALs or promote directly). */ static AutoFailoverNode * -SelectFailoverCandidateNode(CandidateList *candidateList, +SelectFailoverCandidateNode(GroupStateContext *ctx, + CandidateList *candidateList, AutoFailoverNode *primaryNode) { /* @@ -1789,7 +1835,7 @@ SelectFailoverCandidateNode(CandidateList *candidateList, AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); /* all the candidates are now in the REPORT_LSN state */ - if (IsUnhealthy(node)) + if (NodeIsUnhealthy(node, ctx)) { char message[BUFSIZE]; @@ -1840,7 +1886,7 @@ SelectFailoverCandidateNode(CandidateList *candidateList, { AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); - if (IsHealthy(node)) + if (NodeIsHealthy(node, ctx)) { someMostAdvancedStandbysAreHealthy = true; break; diff --git a/src/monitor/group_state_machine.h b/src/monitor/group_state_machine.h index bab91395c..65e9f0fac 100644 --- a/src/monitor/group_state_machine.h +++ b/src/monitor/group_state_machine.h @@ -15,6 +15,8 @@ #include "postgres.h" #include "access/xlogdefs.h" +#include "datatype/timestamp.h" +#include "formation_metadata.h" #include "node_metadata.h" /* @@ -34,8 +36,36 @@ typedef struct AutoFailoverNodeState } AutoFailoverNodeState; +/* + * GroupStateContext bundles every input the monitor node_active protocol needs + * to make FSM decisions: the node list (loaded once from the DB), the + * formation, a single timestamp snapshot, and copies of the relevant GUCs. + * + * Production code builds this with BuildGroupStateContext(), which fetches + * everything from the database. Test code can populate it from fixtures and + * call ProceedGroupStateFromContext() directly, making the FSM logic + * exercisable without a live database. + */ +typedef struct GroupStateContext +{ + char *formationId; + int groupId; + AutoFailoverNode *activeNode; + List *groupNodeList; + int groupNodeCount; + AutoFailoverFormation *formation; + TimestampTz now; + int unhealthyTimeoutMs; + int drainTimeoutMs; + int startupGracePeriodMs; +} GroupStateContext; + + /* public function declarations */ +extern bool BuildGroupStateContext(GroupStateContext *ctx, + AutoFailoverNode *activeNode); extern bool ProceedGroupState(AutoFailoverNode *activeNode); +extern bool ProceedGroupStateFromContext(GroupStateContext *ctx); /* GUCs */ extern int EnableSyncXlogThreshold; diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index 03eafa2b4..83f4453e1 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -26,6 +26,7 @@ #include "access/htup_details.h" #include "access/xlogdefs.h" +#include "utils/timestamp.h" #include "catalog/pg_enum.h" #include "nodes/makefuncs.h" #include "nodes/parsenodes.h" @@ -435,6 +436,7 @@ NodeActive(char *formationId, AutoFailoverNodeState *currentNodeState) else { LockFormation(formationId, ShareLock); + LockNodeGroup(formationId, currentNodeState->groupId, ExclusiveLock); if (pgAutoFailoverNode->reportedState != currentNodeState->replicationState) { @@ -484,9 +486,27 @@ NodeActive(char *formationId, AutoFailoverNodeState *currentNodeState) currentNodeState->pgsrSyncState, currentNodeState->reportedTLI, currentNodeState->reportedLSN); - } - LockNodeGroup(formationId, currentNodeState->groupId, ExclusiveLock); + /* + * Sync the in-memory struct with the values just written to the DB. + * ReportAutoFailoverNodeState updates reportedpgisrunning, reportedtli, + * and reporttime in the DB, but not in this struct. ProceedGroupState + * relies on NodeIsHealthy() which reads pgIsRunning and reportTime from + * the struct, so leaving them stale causes spurious health failures. + * + * Mirror the DB's CASE WHEN 0 THEN reportedtli ELSE $4 END logic for + * reportedTLI: a keeper reporting TLI=0 (e.g. wait_standby before + * streaming starts) must not overwrite the struct's existing value, + * because InsertEvent() passes node->reportedTLI directly and the + * event_reportedtli_check constraint requires reportedtli > 0. + */ + pgAutoFailoverNode->pgIsRunning = currentNodeState->pgIsRunning; + if (currentNodeState->reportedTLI != 0) + { + pgAutoFailoverNode->reportedTLI = currentNodeState->reportedTLI; + } + pgAutoFailoverNode->reportTime = GetCurrentTimestamp(); + } ProceedGroupState(pgAutoFailoverNode); diff --git a/src/monitor/node_metadata.c b/src/monitor/node_metadata.c index 34fb6adc6..5fba2596e 100644 --- a/src/monitor/node_metadata.c +++ b/src/monitor/node_metadata.c @@ -1932,6 +1932,20 @@ IsHealthy(AutoFailoverNode *pgAutoFailoverNode) return pgAutoFailoverNode->pgIsRunning; } + /* + * UNKNOWN (-1) means no health-check worker has run yet. Trust the + * keeper's own pgIsRunning report; we have no contradictory evidence. + * Mirrors the same rule in NodeIsHealthy(). Without this, + * CountHealthyCandidates() rejects healthy secondaries that reached + * SECONDARY state before the health-check worker ran (which our FSM + * now allows via NodeIsHealthy), causing start_maintenance() to + * report "0 candidate nodes available". + */ + if (pgAutoFailoverNode->health == NODE_HEALTH_UNKNOWN) + { + return pgAutoFailoverNode->pgIsRunning; + } + /* nominal case: trust background checks + reported Postgres state */ return pgAutoFailoverNode->health == NODE_HEALTH_GOOD && pgAutoFailoverNode->pgIsRunning == true; @@ -2036,3 +2050,106 @@ IsDrainTimeExpired(AutoFailoverNode *pgAutoFailoverNode) return drainTimeExpired; } + + +/* + * NodeIsHealthy is the context-pure variant of IsHealthy. It uses the + * timestamp snapshot in ctx->now instead of calling GetCurrentTimestamp(). + */ +bool +NodeIsHealthy(const AutoFailoverNode *node, const struct GroupStateContext *ctx) +{ + int nodeActiveCallsFrequencyMs = 1 * 1000; + + if (node == NULL) + { + return false; + } + + if (node->health == NODE_HEALTH_BAD && + TimestampDifferenceExceeds(node->healthCheckTime, node->reportTime, 0) && + !TimestampDifferenceExceeds(node->reportTime, ctx->now, + nodeActiveCallsFrequencyMs)) + { + return node->pgIsRunning; + } + + /* + * UNKNOWN (-1) means no health-check worker has run yet. Trust the + * keeper's own pgIsRunning report; we have no contradictory evidence. + */ + if (node->health == NODE_HEALTH_UNKNOWN) + { + return node->pgIsRunning; + } + + return node->health == NODE_HEALTH_GOOD && node->pgIsRunning; +} + + +/* + * NodeIsUnhealthy is the context-pure variant of IsUnhealthy. + */ +bool +NodeIsUnhealthy(const AutoFailoverNode *node, const struct GroupStateContext *ctx) +{ + if (node == NULL) + { + return true; + } + + if (TimestampDifferenceExceeds(node->reportTime, ctx->now, + ctx->unhealthyTimeoutMs)) + { + if (node->health == NODE_HEALTH_BAD && + TimestampDifferenceExceeds(PgStartTime, node->healthCheckTime, 0)) + { + if (TimestampDifferenceExceeds(PgStartTime, ctx->now, + ctx->startupGracePeriodMs)) + { + return true; + } + } + } + + if (!node->pgIsRunning) + { + return true; + } + + return false; +} + + +/* + * NodeIsReporting is the context-pure variant of IsReporting. + */ +bool +NodeIsReporting(const AutoFailoverNode *node, const struct GroupStateContext *ctx) +{ + if (node == NULL) + { + return false; + } + + return !TimestampDifferenceExceeds(node->reportTime, ctx->now, + ctx->unhealthyTimeoutMs); +} + + +/* + * NodeIsDrainTimeExpired is the context-pure variant of IsDrainTimeExpired. + */ +bool +NodeIsDrainTimeExpired(const AutoFailoverNode *node, + const struct GroupStateContext *ctx) +{ + if (node == NULL || + node->goalState != REPLICATION_STATE_DEMOTE_TIMEOUT) + { + return false; + } + + return TimestampDifferenceExceeds(node->stateChangeTime, ctx->now, + ctx->drainTimeoutMs); +} diff --git a/src/monitor/node_metadata.h b/src/monitor/node_metadata.h index 607cf8bd9..6c7440b6e 100644 --- a/src/monitor/node_metadata.h +++ b/src/monitor/node_metadata.h @@ -241,3 +241,19 @@ extern bool IsHealthy(AutoFailoverNode *pgAutoFailoverNode); extern bool IsUnhealthy(AutoFailoverNode *pgAutoFailoverNode); extern bool IsDrainTimeExpired(AutoFailoverNode *pgAutoFailoverNode); extern bool IsReporting(AutoFailoverNode *pgAutoFailoverNode); + +/* + * Pure (context-based) variants of the health predicates. They take a + * pre-captured GroupStateContext instead of calling GetCurrentTimestamp() or + * reading GUC globals themselves. Forward-declare the struct here to avoid a + * circular dependency between node_metadata.h and group_state_machine.h. + */ +struct GroupStateContext; +extern bool NodeIsHealthy(const AutoFailoverNode *node, + const struct GroupStateContext *ctx); +extern bool NodeIsUnhealthy(const AutoFailoverNode *node, + const struct GroupStateContext *ctx); +extern bool NodeIsReporting(const AutoFailoverNode *node, + const struct GroupStateContext *ctx); +extern bool NodeIsDrainTimeExpired(const AutoFailoverNode *node, + const struct GroupStateContext *ctx); diff --git a/src/monitor/pg_auto_failover.c b/src/monitor/pg_auto_failover.c index e7c468627..80778fe6f 100644 --- a/src/monitor/pg_auto_failover.c +++ b/src/monitor/pg_auto_failover.c @@ -179,7 +179,7 @@ StartMonitorNode(void) "Wait for at least this much time after startup before " "initiating a failover.", NULL, &StartupGracePeriodMs, 10 * 1000, 1, INT_MAX, - PGC_SIGHUP, GUC_UNIT_MS, NULL, NULL, NULL); + PGC_SUSET, GUC_UNIT_MS, NULL, NULL, NULL); PreviousProcessUtility_hook = ProcessUtility_hook; ProcessUtility_hook = pgautofailover_ProcessUtility; diff --git a/src/monitor/sql/node_active_protocol.sql b/src/monitor/sql/node_active_protocol.sql new file mode 100644 index 000000000..860baab35 --- /dev/null +++ b/src/monitor/sql/node_active_protocol.sql @@ -0,0 +1,393 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression tests for the monitor's node_active() protocol. +-- +-- Covers two concrete regressions: +-- +-- #1062 — Primary has health=BAD (health-check worker cannot reach it) but +-- IS calling node_active with pgIsRunning=true. NodeIsHealthy must +-- return true via the fresh-report override so no spurious failover +-- is triggered. +-- +-- #1032 — After a failover the old primary catches up while the health +-- checker still marks it BAD. The fixed NodeIsHealthy() allows +-- catchingup→secondary to proceed. Without the fix the transition +-- was permanently blocked (NodeIsHealthy returned false for any +-- node with health=BAD regardless of the live report). +-- +-- Additional regression: NODE_HEALTH_UNKNOWN (-1) is the initial DB value for +-- newly registered nodes (no health-check worker has run yet). NodeIsHealthy +-- must treat UNKNOWN as "trust the keeper's own pgIsRunning report." This +-- manifests during formation bootstrap: the catchingup→secondary transition +-- requires NodeIsHealthy(joining-node) to be true. +-- +-- Also exercises the stale in-memory struct fix in NodeActive(): after +-- ReportAutoFailoverNodeState writes pgIsRunning to the DB, the same value +-- must be synced back into the pgAutoFailoverNode struct before +-- ProceedGroupState runs, otherwise pgIsRunning appears stale to +-- NodeIsHealthy when the keeper changes from pgIsRunning=false to true. + +\x on + +-- ── formation and node registration ───────────────────────────────────────── + +SELECT pgautofailover.create_formation('fsm_test', 'pgsql', 'postgres', true, 1); + +-- sysidentifier=1: a non-zero value satisfies the same_system_identifier +-- constraint without a separate set_node_system_identifier() call. +SELECT * + FROM pgautofailover.register_node('fsm_test', 'node1', 5432, + 'postgres', 'node1', 1); + +SELECT nodeid AS n1 FROM pgautofailover.node + WHERE formationid = 'fsm_test' AND nodename = 'node1' \gset + +SELECT * + FROM pgautofailover.register_node('fsm_test', 'node2', 5432, + 'postgres', 'node2', 1); + +SELECT nodeid AS n2 FROM pgautofailover.node + WHERE formationid = 'fsm_test' AND nodename = 'node2' \gset + +-- ── formation bootstrap ────────────────────────────────────────────────────── +-- +-- IsCurrentState(node, S) requires both goalState=S and reportedState=S, so +-- every intermediate state must be explicitly confirmed before the next guard +-- fires. The sequence below mirrors what two real keepers would produce. + +-- node1: single (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'single'); + +-- node2: wait_standby (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_standby'); + +-- node1: single → wait_primary (secondary has joined in WAIT_STANDBY) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'single', + current_lsn => '0/5000'); + +-- node1: wait_primary (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +-- node2: wait_standby → catchingup (primary is confirmed in WAIT_PRIMARY) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_standby'); + +-- node2: catchingup → secondary +-- NODE_HEALTH_UNKNOWN (-1) regression: health is -1 (default for new nodes; +-- the health-check worker has not run). NodeIsHealthy must trust the +-- keeper's pgIsRunning=true instead of returning false. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +-- node2: secondary (confirm) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +-- node1: wait_primary → primary (secondary quorum satisfied) +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +-- ── test_001: steady state ─────────────────────────────────────────────────── + +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); + +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +-- ── test_002: issue #1062 ───────────────────────────────────────────────────── +-- +-- Health checker marks primary BAD while the primary is still calling +-- node_active with pgIsRunning=true. NodeIsHealthy must return true via the +-- fresh-report override: +-- health=BAD, healthchecktime < reportTime, reportTime within 1s of now. +-- No spurious failover must be triggered. + +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; + +-- Primary reports pgIsRunning=true; fresh report overrides the BAD health. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_lsn => '0/5000'); + +-- Secondary calls in; NodeIsUnhealthy(primary) is false — no promotion. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/5000'); + +-- restore health before the full-failure test +UPDATE pgautofailover.node + SET health = 1, + healthchecktime = now() + WHERE nodename = 'node1'; + +-- ── test_003: two-node failover ─────────────────────────────────────────────── + +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; + +-- Primary's own heartbeat with pgIsRunning=false. In the two-node case +-- ProceedGroupStateForPrimaryNode has no self-demotion rule; the failover +-- trigger fires from the standby's heartbeat. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => false, + current_lsn => '0/5000'); + +-- Secondary calls in: NodeIsUnhealthy(primary) is true → prepare_promotion. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/5000'); + +-- node2 reports prepare_promotion → assigned stop_replication. +-- node1 gets goalState=demote_timeout. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'prepare_promotion', + current_pg_is_running => true, + current_lsn => '0/5000'); + +-- node1 reports demote_timeout; satisfies IsCurrentState(node1, DEMOTE_TIMEOUT) +-- so that the next node2 heartbeat can advance. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'demote_timeout', + current_pg_is_running => false, + current_lsn => '0/5000'); + +-- node2 reports stop_replication → assigned wait_primary. +-- node1 gets goalState=demoted. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'stop_replication', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- node2 in wait_primary; no secondary in quorum yet. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- ── test_004: issue #1032 ───────────────────────────────────────────────────── +-- +-- Old primary (node1) resurfaces. Health checker is still marking it BAD +-- (recovery in progress). The fixed NodeIsHealthy() must allow the +-- catchingup→secondary transition. +-- +-- The stale in-memory struct fix is also exercised here: node1's previous +-- node_active call reported pgIsRunning=false (demoted). The DB therefore +-- has reportedpgisrunning=false. When node1 reports catchingup with +-- pgIsRunning=true, ReportAutoFailoverNodeState writes true to the DB but the +-- in-memory struct still holds false. Without the fix, ProceedGroupState +-- sees pgIsRunning=false and NodeIsHealthy returns false, permanently blocking +-- the transition. + +UPDATE pgautofailover.node + SET health = 1, + healthchecktime = now() + WHERE nodename = 'node1'; + +-- node1 resurfaces reporting 'primary' while its goalState is 'demoted'. +-- IsCurrentState(node1, DEMOTED) is false (reportedState mismatch), so no +-- DEMOTED rule fires; the monitor returns the current goalState = demoted. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_tli => 1, + current_lsn => '0/4F00'); + +-- node1 reports demoted → catchingup assigned. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'demoted', + current_pg_is_running => false, + current_tli => 1, + current_lsn => '0/4F00'); + +-- Health checker marks node1 BAD again (recovery still in progress). +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now() - interval '1 second' + WHERE nodename = 'node1'; + +-- node1 calls node_active with pgIsRunning=true. +-- NodeIsHealthy(node1) returns true via the fresh-report override. +-- The stale-struct fix ensures the pgIsRunning=true from this call's report +-- is visible to ProceedGroupState within the same call. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'catchingup', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- node1 confirms secondary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- node2 now has a healthy secondary in the quorum → promoted to primary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'wait_primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- ── test_005: start_maintenance on primary with health=UNKNOWN secondary ────── +-- +-- Regression for the CountHealthyCandidates / IsHealthy inconsistency. +-- +-- NodeIsHealthy() treats NODE_HEALTH_UNKNOWN as "trust pgIsRunning", so +-- catchingup→secondary can fire before the health-check worker runs. The +-- old IsHealthy() returned false for UNKNOWN, so start_maintenance() counted +-- 0 healthy candidates even when the secondary was fully reachable. +-- +-- After fixing IsHealthy() to mirror NodeIsHealthy() for UNKNOWN health, +-- start_maintenance must succeed and assign prepare_maintenance / +-- prepare_promotion. + +-- Confirm node2 as primary (last test_004 call left it in wait_primary goal). +SELECT * + FROM pgautofailover.node_active('fsm_test', :n2, 0, + current_group_role => 'primary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- Confirm node1 as secondary. +SELECT * + FROM pgautofailover.node_active('fsm_test', :n1, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_tli => 2, + current_lsn => '0/5000'); + +-- Force node1 health back to UNKNOWN to simulate a node that reached +-- SECONDARY before the health-check worker ran (the window our FSM fix +-- opened up). Without the IsHealthy() fix this causes start_maintenance +-- to fail with "0 candidate nodes available". +UPDATE pgautofailover.node + SET health = -1 + WHERE nodename = 'node1'; + +-- start_maintenance on the primary (node2): must succeed despite +-- node1 health=UNKNOWN, because IsHealthy() now trusts pgIsRunning=true +-- when no health check has run yet. +SELECT pgautofailover.start_maintenance(:n2); + +-- Verify: node2 → prepare_maintenance, node1 → prepare_promotion. +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'fsm_test' + ORDER BY nodename; + +-- ── test_006: killed-primary failover (health=BAD, pgIsRunning still true) ──── +-- +-- The container-killed scenario: the primary stops reporting node_active (so +-- pgIsRunning stays TRUE in the DB from its last heartbeat), the health-check +-- worker eventually marks it BAD, and after unhealthyTimeoutMs the monitor +-- should fire secondary → prepare_promotion even though the primary never +-- self-reported pgIsRunning=false. +-- +-- Distinct from test_003 (self-reported-down path via pgIsRunning=false). +-- Here we exercise the time+health-check-based unhealthy path. +-- +-- Uses a fresh formation to start from a clean state. + +SELECT pgautofailover.create_formation('killed_test', 'pgsql', 'postgres', true, 0); + +SELECT * + FROM pgautofailover.register_node('killed_test', 'ka', 5432, + 'postgres', 'ka', 1); + +SELECT nodeid AS ka FROM pgautofailover.node + WHERE formationid = 'killed_test' AND nodename = 'ka' \gset + +SELECT * + FROM pgautofailover.register_node('killed_test', 'kb', 5432, + 'postgres', 'kb', 1); + +SELECT nodeid AS kb FROM pgautofailover.node + WHERE formationid = 'killed_test' AND nodename = 'kb' \gset + +-- Bootstrap to primary + secondary. +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'single'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'wait_standby'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'single', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'wait_primary', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'wait_standby'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'catchingup', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'secondary', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'wait_primary', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :ka, 0, current_group_role => 'primary', current_lsn => '0/3000'); +SELECT assigned_group_state FROM pgautofailover.node_active('killed_test', :kb, 0, current_group_role => 'secondary', current_lsn => '0/3000'); + +-- Lower startup_grace_period to 1ms so the time-based unhealthy path fires +-- immediately in the test environment (in production the server has been up +-- far longer than the 10s default). +SET pgautofailover.startup_grace_period = 1; + +-- Simulate killed primary (ka): health checker ran and marked it BAD, +-- but ka's last node_active set pgIsRunning=true and that value is still in +-- the DB — it never self-reported pgIsRunning=false. +-- reporttime is set 60s in the past to exceed UnhealthyTimeoutMs (20s default). +-- healthchecktime is recent so the health-check-ran-after-startup condition holds. +UPDATE pgautofailover.node + SET health = 0, + healthchecktime = now(), + reporttime = now() - interval '60 seconds' + WHERE formationid = 'killed_test' AND nodename = 'ka'; + +-- kb (secondary) calls node_active: NodeIsUnhealthy(ka, ctx) must fire via +-- the time-based path (reportTime > unhealthyTimeoutMs AND health=BAD), +-- triggering secondary → prepare_promotion even though ka never called +-- node_active with pgIsRunning=false. +SELECT * + FROM pgautofailover.node_active('killed_test', :kb, 0, + current_group_role => 'secondary', + current_pg_is_running => true, + current_lsn => '0/3000'); + +RESET pgautofailover.startup_grace_period; diff --git a/tests/pgautofailover_utils.py b/tests/pgautofailover_utils.py index 7777e151b..f06000e21 100644 --- a/tests/pgautofailover_utils.py +++ b/tests/pgautofailover_utils.py @@ -946,7 +946,10 @@ def wait_until_state( while wait_until > dt.datetime.now(): self.sleep(sleep_time) - current_state, assigned_state = self.get_state() + try: + current_state, assigned_state = self.get_state() + except Exception: + continue # only log the state if it has changed if current_state != prev_state: @@ -994,7 +997,10 @@ def wait_until_assigned_state( while wait_until > dt.datetime.now(): self.cluster.sleep(sleep_time) - current_state, assigned_state = self.get_state() + try: + current_state, assigned_state = self.get_state() + except Exception: + continue # only log the state if it has changed if assigned_state != prev_state: @@ -1210,9 +1216,14 @@ def create( # sometimes we might have holes in the nodeid sequence # grab the current nodeid, if it's already available - nodeid = self.get_nodeid() - if nodeid > 0: - self.nodeid = nodeid + # when run=True the background process may not have written its state + # file yet — tolerate that and leave self.nodeid at its default + try: + nodeid = self.get_nodeid() + if nodeid > 0: + self.nodeid = nodeid + except CalledProcessError: + pass def logger_name(self): return self.datadir diff --git a/tests/tap/specs/multi_async.pgaf b/tests/tap/specs/multi_async.pgaf index ee4ea1bd8..7be721dc9 100644 --- a/tests/tap/specs/multi_async.pgaf +++ b/tests/tap/specs/multi_async.pgaf @@ -225,7 +225,6 @@ step test_016_002_fail_node1 { step test_016_003_restart_node3 { network connect node3 wait until node3 assigned-state = secondary timeout 90s - wait until node2 state is wait_primary timeout 60s sleep 5s } @@ -234,7 +233,7 @@ step test_016_004_restart_node1 { wait until node3 state is secondary and node2 state is primary and node1 state is secondary - timeout 90s + timeout 120s } step test_017_001_fail_node1 { diff --git a/tests/tap/specs/replace_monitor.pgaf b/tests/tap/specs/replace_monitor.pgaf index 94cef2dd2..8762fb255 100644 --- a/tests/tap/specs/replace_monitor.pgaf +++ b/tests/tap/specs/replace_monitor.pgaf @@ -66,8 +66,6 @@ step enable_monitor_node1 { step enable_monitor_node2 { exec node2 pg_autoctl enable monitor postgresql://autoctl_node@newmonitor/pg_auto_failover - wait until node2 state is catchingup timeout 90s - wait until node1 state is wait_primary timeout 90s } step wait_for_convergence { diff --git a/tests/test_replace_monitor.py b/tests/test_replace_monitor.py index 7c02ea5b9..5069553d8 100644 --- a/tests/test_replace_monitor.py +++ b/tests/test_replace_monitor.py @@ -86,8 +86,6 @@ def test_009a_enable_monitor_node1(): def test_009b_enable_monitor_node2(): node2.enable_monitor(newmonitor) - assert node2.wait_until_state(target_state="catchingup") - assert node1.wait_until_state(target_state="wait_primary") def test_010_wait_until_state():