Fix 3-DC split-brain: monitor assigns wait_primary on replication stall (issue #997)

[dimitri]

Problem

In a 3-DC topology — primary in dc1, standby in dc2, monitor in dc3 — cutting the dc1↔dc2 link while both nodes remain reachable from the monitor leaves synchronous_standby_names set on the primary. Every COMMIT then hangs indefinitely waiting for an acknowledgement from a standby that can no longer be reached.

Closes #997.

Root cause

The FSM had no rule to transition the primary out of the primary state when synchronous replication was stalled. The monitor could see that the standby was unreachable from the primary (via pg_stat_replication reporting sync_state = 'unknown') but took no action.

Fix

Monitor side (src/monitor/):

• Add a replication_stall_since column to pgautofailover.node. The monitor sets it to now() the first time a primary reports sync_state = unknown (replication partner unreachable) and clears it when replication recovers.
• Add a region column to pgautofailover.node (populated from --region on pg_autoctl create postgres). This lets operators label nodes with their DC location for observability.
• Add a GUC pgautofailover.replication_stall_timeout (default 10 s). When now() - replication_stall_since > timeout, the FSM assigns wait_primary, which clears synchronous_standby_names and unblocks writes.
• Extension bumped to 2.3 with an upgrade path from 2.2.

Client side (src/bin/pg_autoctl/):

• --region / -G flag added to pg_autoctl create postgres and wired through keeper_config, monitor_register_node, and nodespec.
• pg_autoctl watch updated to display the region column.

Test (tests/tap/specs/replication_stall_3dc.pgaf):

New end-to-end regression test using the pgaftest framework. The test models the 3-DC scenario in a single Docker Compose stack: the monitor is always reachable from both nodes, but docker network disconnect severs the dc1↔dc2 replication link. Six steps verify:

1. Sync replication is active (synchronous_standby_names = 'ANY 1 (…)')
2. The dc1↔dc2 link is cut
3. The monitor assigns wait_primary within ~20 s (well within the 60 s timeout)
4. synchronous_standby_names is cleared; writes no longer hang
5. Link restored; standby re-joins
6. Row written during the stall has replicated

Verified locally, 6/6 steps passing against PG17.

Watch the fix work, interactively

Bring the exact topology up and drive it step by step (pulls/builds images on first run, ~30s):

pgaftest tmux tests/tap/specs/replication_stall_3dc.pgaf

This opens a 3-pane tmux session: docker compose logs -f (top), pg_autoctl watch — the live FSM state table (middle), an interactive shell (bottom). From the shell:

# See the six steps and where you are
pgaftest show steps

# Preview the DSL commands the next step will run before running it
pgaftest show step

# Confirm sync replication is active: synchronous_standby_names = 'ANY 1 (...)'
pgaftest step

# Sever the dc1<->dc2 link -- watch the middle pane: node1 stays "primary"
# reporting an empty sync_state
pgaftest step
pgaftest show state

# Within ~10-20s (replication_stall_timeout), node1 flips to wait_primary
# in the live watch pane -- this step just asserts what you can already see
pgaftest step

# Confirm writes actually unblocked: synchronous_standby_names is now empty
pgaftest step
pgaftest sql node1 { SHOW synchronous_standby_names; }

# Restore the link; node2 rejoins as secondary, node1 confirms primary
pgaftest step

# Confirm replication caught back up, including the row written during the stall
pgaftest step

# Tear down
pgaftest cluster down

Each pgaftest step (no argument) auto-advances to the next pending step in the spec's sequence; pgaftest show steps at any point lists all six with * marking which one runs next.

To see the bug rather than the fix: the spec file itself doesn't exist before this PR, but the FSM rule it exercises is a single, self-contained if block in ProceedGroupStateFromContext() (src/monitor/group_state_machine.c). Comment that block out, rebuild the monitor image, and rerun the spec — step 3 times out instead of passing (node1 stays at primary with synchronous_standby_names still set), and step 4's write hangs forever (interrupt it and pgaftest cluster down to clean up).

Reusing this for future bug reports

The spec above didn't start as a regression test — it started as the smallest .pgaf file that reliably reproduced the reported bug. That's a generally useful pattern for triaging issues in this repo, not just for this one:

1. When opening an issue, attach a minimal .pgaf file describing just the topology and the manual steps that trigger the problem (a cluster{} block, a setup{} that gets to the interesting state, and network/sql/exec commands reproducing the failure — no step{}/sequence needed yet). pgaftest show compose <spec.pgaf> lets the reporter sanity-check the generated Docker Compose file without needing to actually understand pgaftest's internals.
2. When triaging, a maintainer runs pgaftest tmux <attached-spec.pgaf> and gets the exact reported topology live in under a minute, with pg_autoctl watch showing FSM state in real time and docker compose logs -f showing every node's logs — no need to describe the bug in prose or guess at timing from a text report, and no need to reconstruct the environment from scratch.
3. While diagnosing, pgaftest sql <node> { ... }, pgaftest network disconnect/connect <node>, and pgaftest show state let the maintainer poke at the live cluster interactively — inject the same fault differently, check intermediate state, confirm a hypothesis — without editing the spec or restarting the stack.
4. Once the root cause is understood and fixed, the reporter's manual steps get turned into named step{}s with wait until/assert/expect assertions and a sequence (exactly what happened between the original issue-2 nodes + witness = 3 data centers (problem case detected) #997 repro and replication_stall_3dc.pgaf above), and pgaftest run <spec.pgaf> gives the same scenario a permanent, deterministic, TAP-emitting place in CI — so the exact bug that was reported can never silently come back.

The whole loop — reporter's repro spec straight through to CI regression test — uses the same file and the same tool the whole way, which is the main advantage over a prose bug report: nobody has to translate "here's what I did" into "here's a test" by hand.

Rebase notes

Rebased onto current main (post-#1150) on 2026-07-22; the earlier squashed pgaftest-development commits this branch used to carry (CLI restructure, pg_autoctl watch column-count fix, installcheck expected-output fixes, etc.) are already on main via #1150 and are no longer part of this PR's diff. Two pre-existing bugs were fixed while rebasing: a stray trailing backslash in the Dockerfile that broke every Docker build, and an incomplete 2.2→2.3 version bump (PG_AUTOCTL_EXTENSION_VERSION in defaults.h was left at "2.2", which made every pg_autoctl start try to downgrade the extension). tests/tap/specs/upgrade.pgaf's hardcoded version strings were also updated to match (its PREV_TAG auto-detection now resolves to v2.2 instead of v2.1, since it always targets the newest released tag below the current in-development version).

[tboevil]

Would checking primaryNode->pgsrSyncState be more accurate and timely? And does secondaryQuorumNodesCount need to be updated?

[dimitri]

Thanks for taking a close look at this — both good questions, and worth walking through in detail.

On pgsrSyncState: this is actually already the signal driving the whole mechanism, not an alternative to it. ReportAutoFailoverNodeState()'s update query sets replication_stall_since with:

replication_stall_since = CASE
  WHEN $1 = 'primary' AND $3 = ''   -- $3 is pgsrSyncState
  THEN COALESCE(replication_stall_since, now())
  ELSE NULL
END

pgsrSyncState reported as ''/"unknown" both map to SYNC_STATE_UNKNOWN, so primaryNode->pgsrSyncState == SYNC_STATE_UNKNOWN is exactly the condition that sets replication_stall_since — on the very first report where it's empty, and cleared the instant a standby reappears. On the keeper side, keeper_update_pg_state() re-derives pgsrSyncState from pg_stat_replication.sync_state on every single local report cycle, so it already goes empty immediately when a standby drops out — a walreceiver restart, a brief network blip, anything.

So checking primaryNode->pgsrSyncState directly, as you suggest, would be timelier — but what it actually removes is the debounce: right now we wait for that condition to hold continuously for replication_stall_timeout (10s default) before acting, specifically so a single bad report doesn't flip the primary into wait_primary (and clear synchronous_standby_names) over a hiccup that resolves itself a second later. Reacting on the first empty report would reintroduce exactly the flapping risk the timeout exists to avoid. So: more immediate, yes; more accurate, we'd argue no — same signal, just without the hysteresis.

That said, digging into this did surface a real gap: replication_stall_3dc.pgaf only exercises the sustained-stall path (disconnect, wait out the full timeout, confirm wait_primary) — there's no test confirming a brief disconnect, shorter than replication_stall_timeout, does not trigger it. That's precisely the case the debounce is supposed to handle, and it's currently unverified.

On secondaryQuorumNodesCount: no change needed there. It's computed later in the same function, but the stall check returns early (return true) before that code path runs, and once the primary transitions to wait_primary, IsInPrimaryState(primaryNode) goes false — so the stall check itself stops firing on subsequent ticks. No same-tick or cross-tick interaction with stale data that we could find.

[tboevil]

@dimitri thanks for your reply, I has another question.

Maybe I missed something. we've now added more checks for downgrading (primary -> wait_primary), but we haven't done the corresponding checks for upgrading(like primaryNode->pgsrSyncState != SYNC_STATE_UNKNOWN). Could this lead to repeated oscillations?

[tboevil]

cluster {
    monitor
    formation {
        node1
        node2
    }
}

setup {
    wait until node1 state = primary    timeout 120s
    wait until node2 state = secondary  timeout 120s
}

teardown {
    sql node2 { ALTER SYSTEM RESET primary_conninfo; }
    sql node2 { SELECT pg_reload_conf(); }
    compose down
}

step repro {
    # disable replication connect(192.0.2.0/24 = RFC 5737)
    sql node2 { ALTER SYSTEM SET primary_conninfo = 'host=192.0.2.1 port=5432 user=nobody connect_timeout=2'; }
    sql node2 { SELECT pg_reload_conf(); }

    sleep 30s

    sql monitor {
        SELECT reportedstate || '/' || goalstate
          FROM pgautofailover.node WHERE nodename = 'node1';
    }
    expect { wait_primary/wait_primary }
}

sequence
    repro

There seems to be another problem; reportedrepstate is 'unknown', but it's compared to '', so the condition is never true.

06:05:55 1 INFO  --- end container logs ---
06:05:55 1 INFO  Running teardown block
06:05:55 1 INFO   [01] sql node2 { ALTER SYSTEM RESET primary_conninfo; }
06:05:55 1 INFO   [02] sql node2 { SELECT pg_reload_conf(); }
06:05:56 1 INFO   [03] compose down
06:05:56 1 INFO  Tearing down compose stack (project: replication_stall_standby_healthy)
 Container replication_stall_standby_healthy-node2-1 Stopping
 Container replication_stall_standby_healthy-node2-1 Stopped
 Container replication_stall_standby_healthy-node2-1 Removing
 Container replication_stall_standby_healthy-node2-1 Removed
 Container replication_stall_standby_healthy-node1-1 Stopping
 Container replication_stall_standby_healthy-node1-1 Stopped
 Container replication_stall_standby_healthy-node1-1 Removing
 Container replication_stall_standby_healthy-node1-1 Removed
 Container replication_stall_standby_healthy-monitor-1 Stopping
 Container replication_stall_standby_healthy-monitor-1 Stopped
 Container replication_stall_standby_healthy-monitor-1 Removing
 Container replication_stall_standby_healthy-monitor-1 Removed
 Volume replication_stall_standby_healthy_node1_data Removing
 Network replication_stall_standby_healthy_pgafnet Removing
 Volume replication_stall_standby_healthy_node1_data Removed
 Volume replication_stall_standby_healthy_node2_data Removing
 Volume replication_stall_standby_healthy_monitor_data Removing
 Volume replication_stall_standby_healthy_monitor_data Removed
 Volume replication_stall_standby_healthy_node2_data Removed
 Network replication_stall_standby_healthy_pgafnet Removed
1..1
not ok 1 - repro
# expected "wait_primary/wait_primary", got "primary/primary"
# replication_stall_standby_healthy.pgaf
not ok1        - repro                   30155 ms
1..1
# 1 test failed.

BTW: If you need help (it seems there are a lot of bug fixes lately), I'd be happy to submit a PR after we've finalized a fix.