From f030f9c6550b9c997e33e415d6e8098a2d2d4b77 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 18:08:05 +0000 Subject: [PATCH 01/13] Add Postgres 17+ CDC continuity: synchronized_standby_slots, demotion slot cleanup (#1188) Lets a failover-enabled logical replication slot survive a pg_auto_failover promotion on Postgres 17+, by automating the pieces Postgres itself needs: - Monitor: new pgautofailover.synchronized_standby_slots(formation, group) SQL function, added to the extension's current unreleased schema revision (still '2.3', the last tagged release being v2.2 -- folded into the existing pgautofailover--2.2--2.3.sql migration alongside the other schema changes already accumulated there since v2.2, rather than bumping to a new 2.4; this project only bumps the extension version at actual release time). Computed like synchronous_standby_names but listing every current failover candidate's physical slot name, so a failover-enabled logical slot only advances once every promotable node has replayed that far. - pg_autoctl: fetches and applies that value on the primary via the same fsm_apply_settings() path that already maintains synchronous_standby_names; enables sync_replication_slots by default on 17+ nodes (alongside the existing hot_standby_feedback default), gated by a new pg_setup_synchronized_standby_slots_supported() version check. - Fix issue #1188: a node rejoining as standby after an interrupted demotion could keep its own stale (failover AND NOT synced) logical slots, permanently blocking the slot-sync worker. Extend the existing demotion slot cleanup to drop those too. New `pg_autoctl show synchronized-standby-slots` command (mirrors `show standby-names`), and `pg_autoctl watch`'s header now shows a live "CDC Slots" count next to "Sync Standbys". wal_level stays user-managed, as does logical slot creation itself -- CDC tooling (Debezium, etc) still owns slot lifecycle; pg_auto_failover only keeps the sync infrastructure correct across role changes. Documented in a new docs/cdc.rst page (including a comparison against Patroni, CloudNativePG, EDB PGD, and repmgr's own support for this Postgres feature, and why a single merged CDC stream across Citus workers needs a separate, standalone proxy component rather than an extension of pg_auto_failover itself) and a faq.rst entry. Covered by a new monitor regression test (synchronized_standby_slots.sql) and a pgaftest spec (cdc_continuity.pgaf) exercising a clean failover and the #1188 demote_timeout scenario -- run for real against `pg_auto_failover:pg17` Docker containers via `pgaftest run`, all 5 steps passing. That run surfaced and fixed two issues in the test itself (not the feature): setup{} restarting the primary's Postgres after it already had a synchronous standby attached (the risky pattern basic_operation.pgaf warns about), and a scenario that re-promoted the slot's original creator a second time, which separately hits a genuine Postgres-level limitation (a node round-tripping back to primary can get its own slot stuck in an unresolvable sync loop and lose it) -- now documented as a known limitation rather than conflated with the #1188 regression check. Also verified the real ALTER EXTENSION upgrade path end to end: installed at 2.2, updated to 2.3, confirmed synchronized_standby_slots() is callable immediately after, against a live Postgres 17 instance. --- docs/cdc.rst | 260 ++++++++++++++++++ docs/faq.rst | 32 +++ docs/fault-tolerance.rst | 2 + docs/index.rst | 1 + docs/ref/configuration.rst | 7 + docs/ref/pg_autoctl.rst | 1 + docs/ref/pg_autoctl_show.rst | 1 + ...utoctl_show_synchronized_standby_slots.rst | 100 +++++++ docs/ref/pg_autoctl_watch.rst | 10 + src/bin/common/pgsetup.c | 25 ++ src/bin/common/pgsetup.h | 1 + src/bin/common/pgsql.c | 62 +++++ src/bin/common/pgsql.h | 3 + src/bin/pg_autoctl/cli_common.h | 1 + src/bin/pg_autoctl/cli_root.c | 2 + src/bin/pg_autoctl/cli_show.c | 66 +++++ src/bin/pg_autoctl/fsm_transition.c | 38 ++- src/bin/pg_autoctl/monitor.c | 63 +++++ src/bin/pg_autoctl/monitor.h | 5 + src/bin/pg_autoctl/primary_standby.c | 112 ++++++++ src/bin/pg_autoctl/primary_standby.h | 3 + src/bin/pg_autoctl/watch.c | 69 +++++ src/bin/pg_autoctl/watch.h | 1 + src/monitor/Makefile | 2 +- .../expected/synchronized_standby_slots.out | 210 ++++++++++++++ src/monitor/node_active_protocol.c | 93 +++++++ src/monitor/pgautofailover--2.2--2.3.sql | 102 +++++++ src/monitor/pgautofailover.sql | 26 ++ src/monitor/regress_schedule | 1 + .../sql/synchronized_standby_slots.sql | 152 ++++++++++ tests/tap/schedule | 1 + tests/tap/schedules/multi-misc.sch | 6 + tests/tap/specs/cdc_continuity.pgaf | 155 +++++++++++ 33 files changed, 1611 insertions(+), 2 deletions(-) create mode 100644 docs/cdc.rst create mode 100644 docs/ref/pg_autoctl_show_synchronized_standby_slots.rst create mode 100644 src/monitor/expected/synchronized_standby_slots.out create mode 100644 src/monitor/sql/synchronized_standby_slots.sql create mode 100644 tests/tap/specs/cdc_continuity.pgaf diff --git a/docs/cdc.rst b/docs/cdc.rst new file mode 100644 index 000000000..5e9ada0d8 --- /dev/null +++ b/docs/cdc.rst @@ -0,0 +1,260 @@ +.. _cdc_continuity: + +Change Data Capture (CDC) Continuity +===================================== + +pg_auto_failover manages **physical** replication slots for its own +primary/standby topology: that part of the design has always been there, +and is covered in :ref:`fault_tolerance`. This page is about **logical** +replication slots, the kind used by Change Data Capture (CDC) tools such as +`Debezium `_, ``pg_logical``, or a hand-rolled +consumer built on ``pgoutput`` / ``wal2json``. + +Historically, a logical replication slot was tied to the single Postgres +instance that created it: if that instance was a pg_auto_failover primary +and a failover promoted one of its standbys, the slot simply didn't exist +on the new primary, and the CDC consumer had to be recreated from scratch +(losing its position, and usually requiring a full resync of the +downstream system). This was discussed at length in `issue #400`__ and +`issue #1030`__, back when Postgres itself had no mechanism to keep a +logical slot in sync on a standby. + +__ https://github.com/hapostgres/pg_auto_failover/issues/400 +__ https://github.com/hapostgres/pg_auto_failover/issues/1030 + +Postgres 17 changed this by adding **failover-enabled logical replication +slots**: a slot created with ``failover => true`` gets its position +mirrored to physical standbys by a dedicated background worker (the +*slot-sync worker*), gated by the ``synchronized_standby_slots`` setting on +the primary. When a standby holding such a synced slot gets promoted, +Postgres lets it be used for logical decoding immediately, with its +existing position intact -- no resync, no special handling required from +whoever is managing the promotion. (The slot's ``synced`` column stays +``true`` as a permanent record of its provenance; it is not reset on +promotion, and isn't the thing that governs whether the slot is usable.) + +pg_auto_failover takes advantage of this, automatically, on every node +running Postgres 17 or more recent: + + - **`dbname` in `primary_conninfo`**: the slot-sync worker refuses to + start at all unless the standby's ``primary_conninfo`` carries a + ``dbname`` keyword. pg_auto_failover has included it since the PG17/18/19 + compatibility work landed. + + - **`sync_replication_slots` and `hot_standby_feedback`**: both are part + of the default settings pg_auto_failover writes to every 17+ node's + configuration, so the slot-sync worker is running on every standby + without any manual setup. (``hot_standby_feedback`` was already a + pg_auto_failover default before 17, for unrelated reasons; 17 only adds + ``sync_replication_slots``.) + + - **`synchronized_standby_slots` on the primary**, computed by the monitor + and kept in sync automatically as the formation's topology changes. This + is the setting that makes the *continuity* guarantee hold: it lists the + physical replication slot names of every current failover candidate + (every node with a non-zero candidate priority), so a failover-enabled + logical slot is only allowed to advance once every node that might get + promoted has already replayed that far. Whichever standby ends up + promoted, it is guaranteed to already have everything the logical slot + had confirmed. You can see the computed value at any time with:: + + pg_autoctl show settings + + which includes a ``synchronized_standby_slots`` row next to the + existing ``synchronous_standby_names`` one. + + - **Cleanup of stale slots after an interrupted demotion**: a node that + was briefly primary before being demoted could otherwise be left + holding on to its old, no-longer-synced failover slots, which blocks the + slot-sync worker from ever starting on it (``same name slot ... already + exists on the standby``). pg_auto_failover now drops those leftover + slots as part of the same cleanup step that already resets physical slot + state after a demotion. + +What you still need to do yourself +----------------------------------- + +pg_auto_failover does not create, drop, or otherwise manage logical +replication slots: that remains entirely your CDC tool's responsibility, +exactly as it would be on a Postgres cluster not managed by +pg_auto_failover. Concretely: + + 1. Set ``wal_level = logical`` yourself. pg_auto_failover does not force + this setting on 17+ nodes, since it has real costs (extra WAL volume, + catalog ``xmin`` retention) that only make sense if you are actually + using logical replication. You can set it through ``pg_autoctl config + set``, or directly in ``postgresql.conf``; either way it requires a + restart, same as on any Postgres instance. + + 2. Create your logical replication slot(s) with ``failover => true``, for + example:: + + SELECT pg_create_logical_replication_slot( + 'debezium', 'pgoutput', false, false, true); + + or, if you're using ``CREATE SUBSCRIPTION`` on a logical subscriber:: + + CREATE SUBSCRIPTION ... + WITH (failover = true, create_slot = true, slot_name = '...'); + + 3. Point your CDC consumer at the formation the way you would point any + client application: through the primary. If you use pg_auto_failover's + own application-facing connection string machinery, a failover already + redirects regular clients transparently; the same connection logic + applies to your CDC consumer's connection to the (new) primary. + +How this compares to other HA tools +------------------------------------ + +Postgres 17's native slot-sync mechanism is new enough (2024) that most HA +tooling is still catching up. As of writing, only a handful of tools +compute ``synchronized_standby_slots`` automatically instead of leaving it +to the operator to set and maintain by hand: + +================== ========================== ============================== ========================= =============================== +Tool Computes the setting? Derived from Manages slot-sync GUCs? Handles stale slot on demotion? +================== ========================== ============================== ========================= =============================== +pg_auto_failover Yes failover candidates Yes, on by default Yes (this work, issue #1188) +Patroni Yes (merged, unreleased) synchronous standbys No (left to the user) Not addressed +CloudNativePG Yes (opt-in) all HA standbys Partial (user must enable) No -- open issue +EDB PGD No (own Raft mechanism) n/a n/a n/a +repmgr No n/a n/a n/a +================== ========================== ============================== ========================= =============================== + +A few points worth calling out: + + - Patroni's equivalent (`patroni/patroni#3527 + `_) derives the slot list + from the cluster's *synchronous* standbys rather than its failover + *candidates* -- the two sets aren't always the same, and Patroni's + feature is inert in an async-only cluster. It's also opt-in behind + ``synchronous_mode`` and, as of this writing, merged but not in a + released Patroni version yet. + + - CloudNativePG computes the slot list but still requires the operator to + turn on ``sync_replication_slots`` themselves, and has an open issue + (`cloudnative-pg#9969 + `_) for + exactly the stale-slot-after-demotion problem this PR's issue #1188 fix + addresses -- the same underlying Postgres interaction, independently + discovered in a different project. + + - Neither tool -- nor core Postgres -- offers a quorum/"N of M" knob for + ``synchronized_standby_slots``; see the first bullet under + *Limitations* below. + + - EDB Postgres Distributed (PGD/BDR) solves a different, larger problem + (any node in a multi-master cluster can consume the slot, via its own + Raft-based coordination) and is not a direct comparison; it layers its + own ``failover_slot_scope`` on top of, and independently from, the + native PG17 mechanism. + +Limitations +----------- + + - This only applies to Postgres 17 and more recent, on every node of the + formation. Mixed-version formations (e.g. during a rolling major-version + upgrade) get no continuity guarantee until every node involved in a + potential promotion is on 17+. + + - ``synchronized_standby_slots`` only ever lists nodes with a non-zero + candidate priority. A node with ``candidate_priority = 0`` can still be + promoted manually (a forced failover), and in that case the usual + Postgres trade-off applies: a logical slot's consumer may need to + tolerate replaying already-seen changes, or, in the worst case, be + recreated. + + - Citus formations are not a primary focus of this feature: Citus already + sets ``wal_level = logical`` for its own metadata synchronization needs, + but CDC continuity for Citus worker/coordinator logical slots has not + been specifically tested. See :ref:`cdc_continuity_citus` below for why + Citus needs more than this feature to give CDC consumers a coherent + single stream, and is intentionally out of scope here. + + - **No "N of M" / per-candidate opt-out.** ``synchronized_standby_slots`` + lists *every* current failover candidate unconditionally: Postgres + itself blocks a failover-enabled logical slot from advancing until + *all* of them confirm, with no quorum knob. A permanently lagging or + unreachable candidate therefore permanently stalls logical decoding + (and piles up WAL on the primary) until it catches up, is dropped as a + candidate, or is promoted. This is a known, documented trade-off of the + underlying Postgres feature, not something pg_auto_failover adds; other + tools that compute this setting (Patroni, CloudNativePG) have the same + constraint, since core provides no way to ask for "N of M". If this + bites you in practice, lowering the affected node's candidate priority + to ``0`` removes it from the list (at the cost of no longer being an + automatic failover candidate). + + - **A node flip-flopping back to primary can lose its own + originally-created slot.** If the *same* node that originally created a + ``failover => true`` slot is demoted, then later promoted back to + primary a second time, Postgres's slot-sync worker can get stuck + indefinitely with ``could not synchronize replication slot "...": + Synchronization could lead to data loss`` (comparing the node's own, + already-more-advanced local WAL/catalog position against what the + *other* node's copy of the slot had reached) -- a temporary placeholder + slot ends up active rather than a real persistent one, and does not + survive the second promotion. This is a Postgres-level position + reconciliation limitation specific to round-tripping back to the exact + original creator, not something pg_auto_failover's demotion cleanup + causes or can resolve on its own; it was found while testing this + feature (see ``tests/tap/specs/cdc_continuity.pgaf``) and does not + affect the common case of failing over to a *different* node. If you + hit it, drop and recreate the slot after the second promotion. + +.. _cdc_continuity_citus: + +Citus: a single merged CDC stream is future work +------------------------------------------------- + +Everything above gives CDC continuity **per Postgres node** (coordinator or +worker). It does not solve the problem of giving a CDC consumer a single, +correctly-ordered stream for a *distributed* table whose rows are sharded +across several Citus worker groups, each independently managed by +pg_auto_failover and each with its own ``wal_level=logical``/failover-slot +setup. + +Today, decoding a sharded table means running a separate consumer per +worker and reconciling downstream -- Citus's own guidance for Debezium-style +CDC is exactly that: one connector per worker node, leaving ordering and +cross-shard transaction reassembly to the consumer. A proper merged stream +would need, roughly: + + - **Topology awareness**: tracking ``pg_dist_shard`` / ``pg_dist_placement`` + (or the ``citus_shards`` view) to know which worker streams currently + back a given distributed table, including across shard rebalancing. + + - **Transaction reassembly**: a cross-shard write is split into one local + transaction per worker it touches, usually committed via two-phase + commit under the hood. The per-worker fragments need to be correlated + back into a single logical transaction (grouping by the 2PC global + transaction id used in ``PREPARE TRANSACTION`` / ``COMMIT PREPARED``) + before being handed to the consumer. + + - **Cross-worker ordering**: once transactions are reassembled, they still + need a deterministic global order and a way to tell a consumer "nothing + older than this point remains outstanding on any worker" -- the + watermark / resolved-timestamp pattern used by other distributed + change-feed systems (buffer each worker's stream, advance a merged + watermark only once every worker has confirmed past it, then emit in + order up to that watermark). + + - **Renumbering**: assigning a single monotonic global sequence/LSN to the + reassembled, reordered stream so a downstream consumer gets stable, + dedupable offsets instead of per-worker LSNs that reset independently + on each worker's own failover. + + - **Failover resilience of the merge layer itself**: each worker's own + slot already survives its own failover (this feature); the merge + component additionally needs its own durable checkpoint -- one position + per worker stream -- so it can resume correctly after its own restart, + independently of any single worker's HA event. + +This is a distinct, substantial piece of work -- effectively a standalone +Citus-aware CDC proxy/gateway sitting in front of per-worker logical +streams, not an extension of pg_auto_failover's monitor/keeper -- and is +intentionally left for a separate effort rather than bundled into this one. + +See also :ref:`configuration` for the full list of settings pg_auto_failover +manages, and :ref:`fault_tolerance` for how physical replication slots are +used internally. diff --git a/docs/faq.rst b/docs/faq.rst index 4e1dcd3d5..eec23d285 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -279,3 +279,35 @@ following commands:: $ pg_autoctl disable monitor $ pg_autoctl enable monitor + +Can I use logical replication / CDC tools (Debezium, etc) with pg_auto_failover? +--------------------------------------------------------------------------------- + +Yes, as long as every node in the formation runs **Postgres 17 or more +recent**. See :ref:`cdc_continuity` for the full picture; the short version: + + - pg_auto_failover doesn't create or manage your logical replication + slots itself -- that stays your CDC tool's job, same as with any other + Postgres cluster. + + - What pg_auto_failover does manage, automatically, on Postgres 17+: the + ``synchronized_standby_slots`` setting on the primary (kept in sync with + the current failover candidates), the ``dbname`` Postgres needs in + ``primary_conninfo`` to run its slot-sync worker, and ``sync_replication_slots`` + / ``hot_standby_feedback`` on every standby. Together, these are what let + a logical slot created with ``failover => true`` keep working after a + promotion, instead of being silently left behind on the old primary. + + - You still need to set ``wal_level = logical`` yourself (pg_auto_failover + leaves this one to you, since it isn't needed unless you actually use + logical replication) and create your slot(s) with ``failover => true``. + +On Postgres versions before 17, Postgres itself has no mechanism to keep a +logical slot's position correct on a standby without losing unconsumed +changes, so there is nothing pg_auto_failover can safely do there: a +logical slot on the primary does not survive a failover, the same as +without pg_auto_failover. This was the situation discussed in `issue #400`__ +and `issue #1030`__ before Postgres 17 added slot synchronization. + +__ https://github.com/hapostgres/pg_auto_failover/issues/400 +__ https://github.com/hapostgres/pg_auto_failover/issues/1030 diff --git a/docs/fault-tolerance.rst b/docs/fault-tolerance.rst index ac9da4f87..76d168a6b 100644 --- a/docs/fault-tolerance.rst +++ b/docs/fault-tolerance.rst @@ -1,3 +1,5 @@ +.. _fault_tolerance: + Failover and Fault Tolerance ============================ diff --git a/docs/index.rst b/docs/index.rst index 5f7f8440e..edb2eb624 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -55,6 +55,7 @@ __ https://github.com/hapostgres/pg_auto_failover architecture-multi-standby failover-state-machine fault-tolerance + cdc security .. toctree:: diff --git a/docs/ref/configuration.rst b/docs/ref/configuration.rst index cf554823b..e20c639b1 100644 --- a/docs/ref/configuration.rst +++ b/docs/ref/configuration.rst @@ -229,6 +229,13 @@ Name of the PostgreSQL replication slot used in the streaming replication setup automatically deployed by pg_auto_failover. Replication slots can't be renamed in PostgreSQL. +On Postgres 17 and more recent, pg_auto_failover also maintains the +``synchronized_standby_slots`` Postgres setting on the primary, computed +from the formation's current failover candidates: this is not a +pg_auto_failover configuration key of its own (there is nothing to set), +but it is what lets a failover-enabled logical replication slot survive a +promotion. See :ref:`cdc_continuity` for the full picture. + **replication.maximum_backup_rate** When pg_auto_failover (re-)builds a standby node using the ``pg_basebackup`` diff --git a/docs/ref/pg_autoctl.rst b/docs/ref/pg_autoctl.rst index 86b143f48..a814dbb35 100644 --- a/docs/ref/pg_autoctl.rst +++ b/docs/ref/pg_autoctl.rst @@ -52,6 +52,7 @@ pg_autoctl provides the following commands:: state Prints monitor's state of nodes in a given formation and group settings Print replication settings for a formation from the monitor standby-names Prints synchronous_standby_names for a given group + synchronized-standby-slots Prints synchronized_standby_slots for a given group (Postgres 17+ CDC continuity) timeline Show the timeline history known to the monitor for a group, and each node's position against it file List pg_autoctl internal files (config, state, pid) systemd Print systemd service file for this node diff --git a/docs/ref/pg_autoctl_show.rst b/docs/ref/pg_autoctl_show.rst index 896b94342..215f6eb37 100644 --- a/docs/ref/pg_autoctl_show.rst +++ b/docs/ref/pg_autoctl_show.rst @@ -13,6 +13,7 @@ pg_autoctl show - Show pg_auto_failover information pg_autoctl_show_state pg_autoctl_show_settings pg_autoctl_show_standby_names + pg_autoctl_show_synchronized_standby_slots pg_autoctl_show_timeline pg_autoctl_show_file pg_autoctl_show_systemd diff --git a/docs/ref/pg_autoctl_show_synchronized_standby_slots.rst b/docs/ref/pg_autoctl_show_synchronized_standby_slots.rst new file mode 100644 index 000000000..55e3516a5 --- /dev/null +++ b/docs/ref/pg_autoctl_show_synchronized_standby_slots.rst @@ -0,0 +1,100 @@ +.. _pg_autoctl_show_synchronized_standby_slots: + +pg_autoctl show synchronized-standby-slots +=========================================== + +pg_autoctl show synchronized-standby-slots - Prints synchronized_standby_slots for a given group (Postgres 17+ CDC continuity) + +Synopsis +-------- + +This command prints the current value for the Postgres 17+ +``synchronized_standby_slots`` setting for the primary Postgres server of +the target group (default ``0``) in the target formation (default +``default``), as computed by the monitor: the comma-separated physical +replication slot names of the group's current failover candidates. See +:ref:`cdc_continuity` for the full picture of what this setting is for:: + + usage: pg_autoctl show synchronized-standby-slots [ --pgdata ] --formation --group + + --pgdata path to data directory + --monitor pg_auto_failover Monitor Postgres URL + --formation formation to query, defaults to 'default' + --group group to query formation, defaults to all + --json output data in the JSON format + +Options +------- + +--pgdata + + Location of the Postgres node being managed locally. Defaults to the + environment variable ``PGDATA``. Use ``--monitor`` to connect to a monitor + from anywhere, rather than the monitor URI used by a local Postgres node + managed with ``pg_autoctl``. + +--monitor + + Postgres URI used to connect to the monitor. Must use the ``autoctl_node`` + username and target the ``pg_auto_failover`` database name. It is possible + to show the Postgres URI from the monitor node using the command + :ref:`pg_autoctl_show_uri`. + + Defaults to the value of the environment variable ``PG_AUTOCTL_MONITOR``. + +--formation + + Show the current ``synchronized_standby_slots`` value for the given + formation. Defaults to the ``default`` formation. + +--group + + Show the current ``synchronized_standby_slots`` value for the given group + in the given formation. Defaults to group ``0``. + +--json + + Output a JSON formatted data instead of a table formatted list. + +Environment +----------- + +PGDATA + + Postgres directory location. Can be used instead of the ``--pgdata`` + option. + +PG_AUTOCTL_MONITOR + + Postgres URI to connect to the monitor node, can be used instead of the + ``--monitor`` option. + +XDG_CONFIG_HOME + + The pg_autoctl command stores its configuration files in the standard + place XDG_CONFIG_HOME. See the `XDG Base Directory Specification`__. + + __ https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html + +XDG_DATA_HOME + + The pg_autoctl command stores its internal states files in the standard + place XDG_DATA_HOME, which defaults to ``~/.local/share``. See the `XDG + Base Directory Specification`__. + + __ https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html + +Examples +-------- + +:: + + $ pg_autoctl show synchronized-standby-slots + 'pgautofailover_standby_2,pgautofailover_standby_3' + + $ pg_autoctl show synchronized-standby-slots --json + { + "formation": "default", + "group": 0, + "synchronized_standby_slots": "pgautofailover_standby_2,pgautofailover_standby_3" + } diff --git a/docs/ref/pg_autoctl_watch.rst b/docs/ref/pg_autoctl_watch.rst index 2a18b940b..816d18afd 100644 --- a/docs/ref/pg_autoctl_watch.rst +++ b/docs/ref/pg_autoctl_watch.rst @@ -84,6 +84,16 @@ currently selected formation, the formation replication setting :ref:`number_sync_standbys`, and then in the right most position the current time. +Whenever the group has at least one failover candidate, the header also +shows a "CDC Slots" count: how many physical slot names the +monitor-computed ``synchronized_standby_slots`` setting currently lists for +that group (see :ref:`cdc_continuity`). This is the same list regardless of +whether you actually have a failover-enabled logical replication slot in +use -- it reflects the topology, not live slot activity. Watching this +number drop during a failover is a quick way to notice a candidate dropping +out of the list; see :ref:`pg_autoctl_show_synchronized_standby_slots` for +the full slot name list. + The second section displays one line per node, and each line contains a list of columns that describe the current state for the node. This list can includes the following columns, and which columns are part of the output diff --git a/src/bin/common/pgsetup.c b/src/bin/common/pgsetup.c index 175f02072..ede8c4377 100644 --- a/src/bin/common/pgsetup.c +++ b/src/bin/common/pgsetup.c @@ -1996,6 +1996,31 @@ pg_setup_standby_slot_supported(PostgresSetup *pgSetup, int logLevel) } +/* + * pg_setup_synchronized_standby_slots_supported returns true when the + * target Postgres instance represented in pgSetup is Postgres 17 or more + * recent, the first version that implements the "synchronized_standby_slots" + * GUC and the pg_sync_replication_slots() slot-sync worker that keeps + * failover-enabled logical replication slots usable across a physical + * standby promotion. Before 17, Postgres has no mechanism to keep a logical + * slot's position correct on a standby without losing unconsumed changes, + * so pg_auto_failover's CDC-continuity support only applies to 17+. + */ +bool +pg_setup_synchronized_standby_slots_supported(PostgresSetup *pgSetup) +{ + int pg_version = 0; + + if (!parse_pg_version_string(pgSetup->pg_version, &pg_version)) + { + /* errors have already been logged */ + return false; + } + + return pg_version >= 1700; +} + + /* * pgsetup_parse_hba_level parses a string that represents an HBAEditLevel * value. diff --git a/src/bin/common/pgsetup.h b/src/bin/common/pgsetup.h index f8afcfa18..55afba8e3 100644 --- a/src/bin/common/pgsetup.h +++ b/src/bin/common/pgsetup.h @@ -277,6 +277,7 @@ SSLMode pgsetup_parse_sslmode(const char *sslMode); char * pgsetup_sslmode_to_string(SSLMode sslMode); bool pg_setup_standby_slot_supported(PostgresSetup *pgSetup, int logLevel); +bool pg_setup_synchronized_standby_slots_supported(PostgresSetup *pgSetup); HBAEditLevel pgsetup_parse_hba_level(const char *level); char * pgsetup_hba_level_to_string(HBAEditLevel hbaLevel); diff --git a/src/bin/common/pgsql.c b/src/bin/common/pgsql.c index 3807f41d2..f28d06dc4 100644 --- a/src/bin/common/pgsql.c +++ b/src/bin/common/pgsql.c @@ -1489,6 +1489,37 @@ pgsql_set_synchronous_standby_names(PGSQL *pgsql, } +/* + * pgsql_set_synchronized_standby_slots sets the Postgres 17+ + * synchronized_standby_slots GUC on the local Postgres to the value + * computed on the pg_auto_failover monitor: the comma-separated list of + * physical replication slot names of the current failover candidates. This + * is what makes a failover-enabled logical replication slot wait for every + * promotable standby before its confirmed position advances, so the slot + * stays usable by a CDC consumer across a promotion. + */ +bool +pgsql_set_synchronized_standby_slots(PGSQL *pgsql, + char *synchronized_standby_slots) +{ + char quoted[BUFSIZE] = { 0 }; + GUC setting = { "synchronized_standby_slots", quoted }; + + if (sformat(quoted, BUFSIZE, "'%s'", synchronized_standby_slots) >= BUFSIZE) + { + log_error("Failed to apply the synchronized_standby_slots value \"%s\": " + "pg_autoctl supports values up to %d bytes and this one " + "requires %lu bytes", + synchronized_standby_slots, + BUFSIZE, + (unsigned long) strlen(synchronized_standby_slots)); + return false; + } + + return pgsql_alter_system_set(pgsql, setting); +} + + /* * pgsql_replication_slot_maintain advances the current confirmed position of * the given replication slot up to the given LSN position, create the @@ -1590,6 +1621,37 @@ pgsql_drop_replication_slot(PGSQL *pgsql, const char *slotName) } +/* + * pgsql_drop_stale_failover_slots drops every logical replication slot that + * still has failover enabled but isn't a synced copy (failover AND NOT + * synced): a slot in exactly that state on a node that is becoming a + * standby again is necessarily a stale leftover from when this node was + * primary, not something the slot-sync worker created. Left in place, it + * blocks that worker from starting at all ("same name slot ... already + * exists on the standby"), so the standby never gets a working synced copy + * (see issue #1188: this happens whenever demotion is interrupted, e.g. by + * demote_timeout, so the normal demotion cleanup below doesn't get a chance + * to run first). + * + * The failover/synced columns only exist from Postgres 17 onward (the + * version that introduces failover-enabled logical slots and the slot-sync + * worker in the first place), so this must only ever be called when + * pg_setup_synchronized_standby_slots_supported() is true. + */ +bool +pgsql_drop_stale_failover_slots(PGSQL *pgsql) +{ + char *sql = + "SELECT pg_drop_replication_slot(slot_name) " + " FROM pg_replication_slots " + " WHERE failover AND NOT synced AND NOT active"; + + log_trace("pgsql_drop_stale_failover_slots"); + + return pgsql_execute_with_params(pgsql, sql, 0, NULL, NULL, NULL, NULL); +} + + /* * BuildNodesArrayValues build the SQL expression to use in a FROM clause to * represent the list of other standby nodes from the given nodeArray. diff --git a/src/bin/common/pgsql.h b/src/bin/common/pgsql.h index f27c6f6e0..b7b004dec 100644 --- a/src/bin/common/pgsql.h +++ b/src/bin/common/pgsql.h @@ -375,9 +375,12 @@ bool pgsql_replication_slot_exists(PGSQL *pgsql, const char *slotName, bool *slotExists); bool pgsql_create_replication_slot(PGSQL *pgsql, const char *slotName); bool pgsql_drop_replication_slot(PGSQL *pgsql, const char *slotName); +bool pgsql_drop_stale_failover_slots(PGSQL *pgsql); bool postgres_sprintf_replicationSlotName(int64_t nodeId, char *slotName, int size); bool pgsql_set_synchronous_standby_names(PGSQL *pgsql, char *synchronous_standby_names); +bool pgsql_set_synchronized_standby_slots(PGSQL *pgsql, + char *synchronized_standby_slots); bool pgsql_replication_slot_create_and_drop(PGSQL *pgsql, NodeAddressArray *nodeArray); bool pgsql_replication_slot_maintain(PGSQL *pgsql, NodeAddressArray *nodeArray); diff --git a/src/bin/pg_autoctl/cli_common.h b/src/bin/pg_autoctl/cli_common.h index 78b7b051c..5892ec575 100644 --- a/src/bin/pg_autoctl/cli_common.h +++ b/src/bin/pg_autoctl/cli_common.h @@ -141,6 +141,7 @@ extern CommandLine show_state_command; extern CommandLine show_settings_command; extern CommandLine show_file_command; extern CommandLine show_standby_names_command; +extern CommandLine show_synchronized_standby_slots_command; extern CommandLine show_timeline_command; /* cli_watch.c */ diff --git a/src/bin/pg_autoctl/cli_root.c b/src/bin/pg_autoctl/cli_root.c index d1c16a587..1d4ba0731 100644 --- a/src/bin/pg_autoctl/cli_root.c +++ b/src/bin/pg_autoctl/cli_root.c @@ -46,6 +46,7 @@ CommandLine *show_subcommands_with_debug[] = { &show_state_command, &show_settings_command, &show_standby_names_command, + &show_synchronized_standby_slots_command, &show_timeline_command, &show_file_command, &systemd_cat_service_file_command, @@ -63,6 +64,7 @@ CommandLine *show_subcommands[] = { &show_state_command, &show_settings_command, &show_standby_names_command, + &show_synchronized_standby_slots_command, &show_timeline_command, &show_file_command, &systemd_cat_service_file_command, diff --git a/src/bin/pg_autoctl/cli_show.c b/src/bin/pg_autoctl/cli_show.c index 0cf0c4b33..5b298493a 100644 --- a/src/bin/pg_autoctl/cli_show.c +++ b/src/bin/pg_autoctl/cli_show.c @@ -46,6 +46,8 @@ static void cli_show_events(int argc, char **argv); static int cli_show_standby_names_getopts(int argc, char **argv); static void cli_show_standby_names(int argc, char **argv); +static void cli_show_synchronized_standby_slots(int argc, char **argv); + static int cli_show_timeline_getopts(int argc, char **argv); static void cli_show_timeline(int argc, char **argv); @@ -128,6 +130,19 @@ CommandLine show_standby_names_command = cli_show_standby_names_getopts, cli_show_standby_names); +CommandLine show_synchronized_standby_slots_command = + make_command("synchronized-standby-slots", + "Prints synchronized_standby_slots for a given group " + "(Postgres 17+ CDC continuity)", + " [ --pgdata ] --formation --group", + " --pgdata path to data directory \n" + " --monitor show the monitor uri\n" + " --formation formation to query, defaults to 'default'\n" + " --group group to query formation, defaults to all\n" + " --json output data in the JSON format\n", + cli_show_standby_names_getopts, + cli_show_synchronized_standby_slots); + CommandLine show_timeline_command = make_command("timeline", "Show the timeline history known to the monitor for a group, " @@ -923,6 +938,57 @@ cli_show_standby_names(int argc, char **argv) } +/* + * cli_show_synchronized_standby_slots prints the Postgres 17+ + * synchronized_standby_slots setting value for a given group (in a known + * formation): the comma-separated physical slot names of the group's + * current failover candidates, which is what lets a failover-enabled + * logical replication slot survive a promotion. See docs/cdc.rst. + */ +static void +cli_show_synchronized_standby_slots(int argc, char **argv) +{ + KeeperConfig config = keeperOptions; + Monitor monitor = { 0 }; + char synchronized_standby_slots[BUFSIZE] = { 0 }; + + (void) cli_monitor_init_from_option_or_config(&monitor, &config); + + (void) cli_set_groupId(&monitor, &config); + + if (!monitor_synchronized_standby_slots( + &monitor, + config.formation, + config.groupId, + synchronized_standby_slots, + BUFSIZE)) + { + log_fatal("Failed to get the synchronized_standby_slots setting value " + " from the monitor, see above for details"); + exit(EXIT_CODE_MONITOR); + } + + if (outputJSON) + { + JSON_Value *js = json_value_init_object(); + JSON_Object *jsObj = json_value_get_object(js); + + json_object_set_string(jsObj, "formation", config.formation); + json_object_set_number(jsObj, "group", (double) config.groupId); + json_object_set_string(jsObj, + "synchronized_standby_slots", + synchronized_standby_slots); + + (void) cli_pprint_json(js); + } + else + { + /* current synchronized_standby_slots might be an empty string */ + (void) fformat(stdout, "'%s'\n", synchronized_standby_slots); + } +} + + /* * cli_show_timeline_getopts parses the command line options for the * command `pg_autoctl show timeline`. diff --git a/src/bin/pg_autoctl/fsm_transition.c b/src/bin/pg_autoctl/fsm_transition.c index 106762a8a..c2778d4cd 100644 --- a/src/bin/pg_autoctl/fsm_transition.c +++ b/src/bin/pg_autoctl/fsm_transition.c @@ -670,7 +670,43 @@ fsm_apply_settings(Keeper *keeper) sizeof(postgres->synchronousStandbyNames)); } - return primary_set_synchronous_standby_names(postgres); + if (!primary_set_synchronous_standby_names(postgres)) + { + /* errors have already been logged */ + return false; + } + + /* + * Also fetch and apply the synchronized_standby_slots setting (Postgres + * 17+ CDC continuity): same monitor round-trip pattern as + * synchronous_standby_names above, computed from the group's current + * failover candidates rather than its synchronous replication quorum. + * primary_set_synchronized_standby_slots() is itself a no-op before + * Postgres 17, so we always fetch the value and let it decide. + */ + if (!config->monitorDisabled) + { + if (!monitor_synchronized_standby_slots( + monitor, + config->formation, + keeper->state.current_group, + postgres->synchronizedStandbySlots, + sizeof(postgres->synchronizedStandbySlots))) + { + log_error("Failed to set synchronized_standby_slots because " + "we failed to get its value from the monitor, " + "see above for details"); + return false; + } + } + else + { + /* no monitor: nothing to synchronize against */ + strlcpy(postgres->synchronizedStandbySlots, "", + sizeof(postgres->synchronizedStandbySlots)); + } + + return primary_set_synchronized_standby_slots(postgres); } diff --git a/src/bin/pg_autoctl/monitor.c b/src/bin/pg_autoctl/monitor.c index f24c34643..48cb0596f 100644 --- a/src/bin/pg_autoctl/monitor.c +++ b/src/bin/pg_autoctl/monitor.c @@ -4061,6 +4061,69 @@ monitor_synchronous_standby_names(Monitor *monitor, } +/* + * monitor_synchronized_standby_slots returns the value for the Postgres 17+ + * parameter "synchronized_standby_slots" to use for a given group, computed + * on the monitor from the current failover candidates in that group (see + * pgautofailover.synchronized_standby_slots() and its C implementation, + * synchronized_standby_slots() in node_active_protocol.c, for the details). + * This is what lets a failover-enabled logical replication slot survive a + * promotion: the primary only lets such slots advance once every node that + * might get promoted has replayed that far. + */ +bool +monitor_synchronized_standby_slots(Monitor *monitor, + char *formation, int groupId, + char *synchronized_standby_slots, int size) +{ + PGSQL *pgsql = &monitor->pgsql; + SingleValueResultContext context = { { 0 }, PGSQL_RESULT_STRING, false }; + + const char *sql = + "select pgautofailover.synchronized_standby_slots($1, $2)"; + + int paramCount = 2; + Oid paramTypes[2] = { TEXTOID, INT4OID }; + const char *paramValues[2] = { 0 }; + IntString myGroupIdString = intToString(groupId); + + paramValues[0] = formation; + paramValues[1] = myGroupIdString.strValue; + + if (!pgsql_execute_with_params(pgsql, sql, + paramCount, paramTypes, paramValues, + &context, &parseSingleValueResult)) + { + log_error("Failed to get the synchronized_standby_slots setting value " + " from the monitor for formation %s and group %d", + formation, groupId); + if (context.strVal) + { + free(context.strVal); + } + return false; + } + + if (!context.parsedOk) + { + log_error("Failed to get the synchronized_standby_slots setting value " + " from the monitor for formation %s and group %d," + "see above for details", + formation, groupId); + if (context.strVal) + { + free(context.strVal); + } + return false; + } + + strlcpy(synchronized_standby_slots, context.strVal, size); + free(context.strVal); + + return true; +} + + /* * monitor_set_hostname sets the hostname on the monitor, using a simple SQL * update command. diff --git a/src/bin/pg_autoctl/monitor.h b/src/bin/pg_autoctl/monitor.h index 3b0fbe770..d4960def8 100644 --- a/src/bin/pg_autoctl/monitor.h +++ b/src/bin/pg_autoctl/monitor.h @@ -279,6 +279,11 @@ bool monitor_synchronous_standby_names(Monitor *monitor, char *synchronous_standby_names, int size); +bool monitor_synchronized_standby_slots(Monitor *monitor, + char *formation, int groupId, + char *synchronized_standby_slots, + int size); + bool monitor_update_node_metadata(Monitor *monitor, int64_t nodeId, const char *name, diff --git a/src/bin/pg_autoctl/primary_standby.c b/src/bin/pg_autoctl/primary_standby.c index 7510fa132..409b9a34d 100644 --- a/src/bin/pg_autoctl/primary_standby.c +++ b/src/bin/pg_autoctl/primary_standby.c @@ -86,6 +86,22 @@ GUC postgres_default_settings_13[] = { { NULL, NULL } }; +/* + * Postgres 17 adds the slot-sync worker (sync_replication_slots) that keeps + * failover-enabled logical replication slots in sync on a physical standby, + * which is what lets a CDC consumer's slot survive promotion. It is only + * useful together with hot_standby_feedback (already on above) and a + * primary_conninfo that includes dbname (handled separately in + * prepare_recovery_settings()); it is a no-op when no failover slot exists, + * so it's safe to enable unconditionally on every 17+ node rather than + * behind a separate opt-in setting. + */ +GUC postgres_default_settings_17[] = { + DEFAULT_GUC_SETTINGS_FOR_PG_AUTO_FAILOVER_13, + { "sync_replication_slots", "on" }, + { NULL, NULL } +}; + GUC citus_default_settings_pre_13[] = { DEFAULT_GUC_SETTINGS_FOR_PG_AUTO_FAILOVER_PRE_13, { "shared_preload_libraries", "'citus,pg_stat_statements'" }, @@ -108,6 +124,18 @@ GUC citus_default_settings_13[] = { { NULL, NULL } }; +GUC citus_default_settings_17[] = { + DEFAULT_GUC_SETTINGS_FOR_PG_AUTO_FAILOVER_13, + { "shared_preload_libraries", "'citus,pg_stat_statements'" }, + { "wal_level", "logical" }, + { "citus.node_conninfo", "'sslmode=prefer'" }, + { "citus.cluster_name", "'default'" }, + { "citus.use_secondary_nodes", "'never'" }, + { "citus.local_hostname", "'localhost'" }, + { "sync_replication_slots", "on" }, + { NULL, NULL } +}; + /* * local_postgres_init_internal initializes an interface for managing a local @@ -705,6 +733,26 @@ primary_drop_all_replication_slots(LocalPostgresServer *postgres) return false; } + /* + * Postgres 17+ CDC continuity (issue #1188): the query above only ever + * touches physical slots matching our own naming pattern, so a logical + * failover slot this node still owns from being primary survives it + * untouched. If that slot is a stale (failover AND NOT synced) leftover + * -- which is exactly what it is, now that we're becoming a standby -- + * it blocks the slot-sync worker from ever starting, so drop it here + * too. This can be reached either after a clean demotion or, via + * fsm_rewind_or_init(), after one interrupted by demote_timeout. + */ + if (pg_setup_synchronized_standby_slots_supported(&(postgres->postgresSetup))) + { + if (!postgres_drop_stale_failover_slots(postgres)) + { + log_error("Failed to drop stale failover replication slots on " + "the local Postgres instance, see above for details"); + return false; + } + } + return true; } @@ -729,6 +777,26 @@ postgres_replication_slot_create_and_drop(LocalPostgresServer *postgres, } +/* + * postgres_drop_stale_failover_slots drops stale (failover AND NOT synced) + * logical replication slots on the local Postgres instance. See + * pgsql_drop_stale_failover_slots() and primary_drop_all_replication_slots() + * (issue #1188) for the rationale. + */ +bool +postgres_drop_stale_failover_slots(LocalPostgresServer *postgres) +{ + PGSQL *pgsql = &(postgres->sqlClient); + + log_trace("postgres_drop_stale_failover_slots"); + + bool result = pgsql_drop_stale_failover_slots(pgsql); + + pgsql_finish(pgsql); + return result; +} + + /* * postgres_replication_slot_advance advances the current confirmed position of * the given replication slot up to the given LSN position. @@ -769,6 +837,39 @@ primary_set_synchronous_standby_names(LocalPostgresServer *postgres) } +/* + * primary_set_synchronized_standby_slots sets the Postgres 17+ + * synchronized_standby_slots GUC on a primary postgres node, so that + * failover-enabled logical replication slots (CDC) survive a promotion. + * + * Postgres versions before 17 don't have this GUC at all, so we skip + * applying it there rather than fail an ALTER SYSTEM SET against an unknown + * parameter name. + */ +bool +primary_set_synchronized_standby_slots(LocalPostgresServer *postgres) +{ + PGSQL *pgsql = &(postgres->sqlClient); + + if (!pg_setup_synchronized_standby_slots_supported(&(postgres->postgresSetup))) + { + log_debug("Skipping synchronized_standby_slots: " + "not supported before Postgres 17"); + return true; + } + + log_info("Setting synchronized_standby_slots to '%s'", + postgres->synchronizedStandbySlots); + + bool result = + pgsql_set_synchronized_standby_slots(pgsql, + postgres->synchronizedStandbySlots); + + pgsql_finish(pgsql); + return result; +} + + /* * primary_disable_synchronous_replication disables synchronous replication * on a primary postgres node. @@ -842,6 +943,17 @@ postgres_add_default_settings(LocalPostgresServer *postgres, default_settings = postgres_default_settings_pre_13; } } + else if (pg_setup_synchronized_standby_slots_supported(pgSetup)) + { + if (IS_CITUS_INSTANCE_KIND(postgres->pgKind)) + { + default_settings = citus_default_settings_17; + } + else + { + default_settings = postgres_default_settings_17; + } + } else { if (IS_CITUS_INSTANCE_KIND(postgres->pgKind)) diff --git a/src/bin/pg_autoctl/primary_standby.h b/src/bin/pg_autoctl/primary_standby.h index d3bc06c64..a0365fbfc 100644 --- a/src/bin/pg_autoctl/primary_standby.h +++ b/src/bin/pg_autoctl/primary_standby.h @@ -78,6 +78,7 @@ typedef struct LocalPostgresServer LocalExpectedPostgresStatus expectedPgStatus; char standbyTargetLSN[PG_LSN_MAXLENGTH]; char synchronousStandbyNames[BUFSIZE]; + char synchronizedStandbySlots[BUFSIZE]; PostgresVersionInfo pgVersion; } LocalPostgresServer; @@ -108,8 +109,10 @@ bool primary_drop_replication_slot(LocalPostgresServer *postgres, bool primary_drop_replication_slots(LocalPostgresServer *postgres); bool primary_drop_all_replication_slots(LocalPostgresServer *postgres); bool primary_set_synchronous_standby_names(LocalPostgresServer *postgres); +bool primary_set_synchronized_standby_slots(LocalPostgresServer *postgres); bool postgres_replication_slot_create_and_drop(LocalPostgresServer *postgres, NodeAddressArray *nodeArray); +bool postgres_drop_stale_failover_slots(LocalPostgresServer *postgres); bool postgres_replication_slot_maintain(LocalPostgresServer *postgres, NodeAddressArray *nodeArray); bool primary_disable_synchronous_replication(LocalPostgresServer *postgres); diff --git a/src/bin/pg_autoctl/watch.c b/src/bin/pg_autoctl/watch.c index 9426d107d..72d02d47d 100644 --- a/src/bin/pg_autoctl/watch.c +++ b/src/bin/pg_autoctl/watch.c @@ -277,6 +277,48 @@ cli_watch_update_from_monitor(WatchContext *context) return false; } + /* + * Postgres 17+ CDC continuity (see docs/cdc.rst): count how many + * physical slots the group's synchronized_standby_slots currently + * lists, same group/formation the rest of this dashboard is showing. + * This is purely topology-derived on the monitor side, so it's safe + * (and cheap) to always fetch, whether or not any node is running + * Postgres 17 or holds a failover-enabled logical slot. + */ + { + char synchronizedStandbySlots[BUFSIZE] = { 0 }; + + if (!monitor_synchronized_standby_slots( + monitor, + context->formation, + context->groupId, + synchronizedStandbySlots, + BUFSIZE)) + { + /* errors have already been logged */ + return false; + } + + if (IS_EMPTY_STRING_BUFFER(synchronizedStandbySlots)) + { + context->synchronizedStandbySlotsCount = 0; + } + else + { + /* comma-separated slot names: N commas means N+1 slots */ + int count = 1; + + for (char *p = synchronizedStandbySlots; *p != '\0'; p++) + { + if (*p == ',') + { + ++count; + } + } + context->synchronizedStandbySlotsCount = count; + } + } + if (!monitor_get_last_events(monitor, context->formation, context->groupId, @@ -639,6 +681,33 @@ print_watch_header(WatchContext *context, int r) mvprintw(r, c, "%d", context->number_sync_standbys); attroff(A_BOLD); + { + char nssStr[12] = { 0 }; + + sformat(nssStr, sizeof(nssStr), "%d", context->number_sync_standbys); + c += strlen(nssStr); + } + + /* + * Postgres 17+ CDC continuity (docs/cdc.rst): show how many physical + * slots synchronized_standby_slots currently lists for this group, so + * an operator watching a live failover can see at a glance whether a + * candidate dropping out (count going down) could be stalling logical + * decoding. Zero is the common, uninteresting case (no CDC in use, or + * no promotion candidates), so it isn't worth the row's space unless + * there's something to see. + */ + if (context->synchronizedStandbySlotsCount > 0 && + context->cols > (c + 9 + 14 + 3)) + { + mvprintw(r, c, " - CDC Slots: "); /* that's 14 chars */ + c += 14; + + attron(A_BOLD); + mvprintw(r, c, "%d", context->synchronizedStandbySlotsCount); + attroff(A_BOLD); + } + /* we only use one row */ return 1; } diff --git a/src/bin/pg_autoctl/watch.h b/src/bin/pg_autoctl/watch.h index 0a2e1e748..74dc6bf9d 100644 --- a/src/bin/pg_autoctl/watch.h +++ b/src/bin/pg_autoctl/watch.h @@ -66,6 +66,7 @@ typedef struct WatchContext char formation[NAMEDATALEN]; int groupId; int number_sync_standbys; + int synchronizedStandbySlotsCount; /* Postgres 17+ CDC continuity, see docs/cdc.rst */ /* data to display */ CurrentNodeStateArray nodesArray; diff --git a/src/monitor/Makefile b/src/monitor/Makefile index a86986656..9715751f4 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 = -Wall -Werror -Wno-unused-parameter -Iinclude -I$(libpq_srcdir) -g SHLIB_LINK = $(libpq) -REGRESS = create_extension monitor workers node_active_protocol guard_data_loss fast_forward drop_node stale_primary_report candidate_count_gate lock_and_fetch_migration timeline_fork_detection dummy_update drop_extension upgrade +REGRESS = create_extension monitor workers node_active_protocol guard_data_loss fast_forward drop_node stale_primary_report candidate_count_gate lock_and_fetch_migration timeline_fork_detection synchronized_standby_slots dummy_update drop_extension upgrade ISOLATION = concurrent_remove_node concurrent_remove_standby_and_primary_report concurrent_remove_standby_and_standby_report concurrent_second_primary_death_report concurrent_health_check_and_report concurrent_candidate_priority_and_quorum PG_CONFIG ?= pg_config diff --git a/src/monitor/expected/synchronized_standby_slots.out b/src/monitor/expected/synchronized_standby_slots.out new file mode 100644 index 000000000..34480ecab --- /dev/null +++ b/src/monitor/expected/synchronized_standby_slots.out @@ -0,0 +1,210 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression test for pgautofailover.synchronized_standby_slots(), the +-- Postgres 17+ CDC-continuity companion to synchronous_standby_names: it +-- lists the physical replication slot names of the group's current +-- failover candidates (AutoFailoverNode.candidatePriority > 0), so that a +-- failover-enabled logical replication slot only advances once every node +-- that might get promoted has replayed that far. +-- +-- Same bootstrap sequence as candidate_count_gate.sql: drive the FSM to +-- primary + secondary + secondary through pgautofailover.node_active(). +-- +-- Unlike candidate_count_gate.sql, register_node()'s result is deliberately +-- NOT printed with "SELECT *": assigned_node_id is a raw sequence value +-- that depends on how many nodes every earlier test in regress_schedule +-- has already registered, which would make this file's expected output +-- fragile to the schedule's exact composition. Only assigned_group_state, +-- which is what this test actually cares about, is printed; node ids are +-- captured silently via \gset for use in node_active() below. +\x on +-- ── formation and node registration ───────────────────────────────────────── +SELECT pgautofailover.create_formation('sss_test', 'pgsql', 'postgres', true, 1); +-[ RECORD 1 ]----+------------------------------ +create_formation | (sss_test,pgsql,postgres,t,1) + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_p', 5432, + 'postgres', 'sss_p', 1); +-[ RECORD 1 ]--------+------- +assigned_group_state | single + +SELECT nodeid AS np FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_p' \gset +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_s1', 5432, + 'postgres', 'sss_s1', 1); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_standby + +SELECT nodeid AS ns1 FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s1' \gset +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_s2', 5432, + 'postgres', 'sss_s2', 1); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_standby + +SELECT nodeid AS ns2 FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s2' \gset +-- ── bootstrap: drive the FSM to primary + secondary + secondary ───────────── +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'single'); +-[ RECORD 1 ]--------+------- +assigned_group_state | single + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_standby + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'single', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+------------- +assigned_group_state | wait_primary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+-------- +assigned_group_state | primary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+---------- +assigned_group_state | secondary + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'wait_standby'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_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('sss_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+--------------- +assigned_group_state | apply_settings + +-- Verify bootstrap: p=primary, s1=secondary, s2=secondary. +SELECT nodename, goalstate, reportedstate, candidatepriority + FROM pgautofailover.node + WHERE formationid = 'sss_test' + ORDER BY nodename; +-[ RECORD 1 ]-----+--------------- +nodename | sss_p +goalstate | apply_settings +reportedstate | primary +candidatepriority | 100 +-[ RECORD 2 ]-----+--------------- +nodename | sss_s1 +goalstate | secondary +reportedstate | secondary +candidatepriority | 100 +-[ RECORD 3 ]-----+--------------- +nodename | sss_s2 +goalstate | catchingup +reportedstate | secondary +candidatepriority | 100 + +-- ── both standbys are candidates: both slot names, in candidatePriority order +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-[ RECORD 1 ]--------------+---------------------------------------------------- +synchronized_standby_slots | pgautofailover_standby_39,pgautofailover_standby_40 + +-- ── s2 opts out of the failover quorum: only s1's slot remains ───────────── +SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s2', 0); +-[ RECORD 1 ]---------------+-- +set_node_candidate_priority | t + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-[ RECORD 1 ]--------------+-------------------------- +synchronized_standby_slots | pgautofailover_standby_39 + +-- ── no candidates left at all: empty string disables the feature ─────────── +SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s1', 0); +NOTICE: setting candidate priority to zero, preventing automated failover +DETAIL: Group 0 in formation "sss_test" have no failover candidate. +-[ RECORD 1 ]---------------+-- +set_node_candidate_priority | t + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-[ RECORD 1 ]--------------+- +synchronized_standby_slots | + +-- ── a single-node group has nothing to synchronize against ───────────────── +SELECT pgautofailover.create_formation('sss_single', 'pgsql', 'postgres', true, 1); +-[ RECORD 1 ]----+-------------------------------- +create_formation | (sss_single,pgsql,postgres,t,1) + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_single', 'sss_only', 5432, + 'postgres', 'sss_only', 1); +-[ RECORD 1 ]--------+------- +assigned_group_state | single + +SELECT pgautofailover.synchronized_standby_slots('sss_single', 0); +-[ RECORD 1 ]--------------+- +synchronized_standby_slots | + +-- ── an empty group errors out, same as synchronous_standby_names ────────── +SELECT pgautofailover.synchronized_standby_slots('sss_test', 42); +ERROR: no nodes found in group 42 of formation "sss_test" diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index a1e22db01..6a7095839 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -71,6 +71,7 @@ PG_FUNCTION_INFO_V1(set_node_replication_quorum); PG_FUNCTION_INFO_V1(set_node_region); PG_FUNCTION_INFO_V1(report_postgres_version); PG_FUNCTION_INFO_V1(synchronous_standby_names); +PG_FUNCTION_INFO_V1(synchronized_standby_slots); PG_FUNCTION_INFO_V1(testing_lock_formation); PG_FUNCTION_INFO_V1(testing_lock_node_group); PG_FUNCTION_INFO_V1(testing_set_node_health); @@ -2647,6 +2648,98 @@ synchronous_standby_names(PG_FUNCTION_ARGS) } +/* + * synchronized_standby_slots returns the value to use for the Postgres 17+ + * "synchronized_standby_slots" parameter on the primary of a given Postgres + * service group in a given formation. + * + * synchronized_standby_slots lists the physical replication slot names that + * a logical replication slot created with failover => true must wait on + * (via the slot's synced copy catching up) before its confirmed_flush_lsn is + * allowed to advance. This is what lets a CDC consumer's logical slot + * survive a failover: as long as every node that might get promoted has + * already replayed everything the logical slot has confirmed, promoting it + * can never leave the slot pointing past data that node doesn't have. + * + * We compute this the same way as synchronous_standby_names above, but + * listing every current failover *candidate* (AutoFailoverNode. + * candidatePriority > 0) rather than only the nodes currently part of the + * synchronous replication quorum: a node can be promoted whether or not it + * was ever a synchronous standby, so the logical slot must wait on it + * regardless. Nodes with candidatePriority == 0 can only be promoted + * manually (force-failover), which is already an exceptional, operator- + * driven action, so we don't hold back CDC consumers for them. + * + * Returns the empty string when there is nothing to wait on (a single node + * group, or no promotion candidates at all), which disables the feature: + * Postgres treats an empty synchronized_standby_slots as "no requirement". + */ +Datum +synchronized_standby_slots(PG_FUNCTION_ARGS) +{ + checkPgAutoFailoverVersion(); + + text *formationIdText = PG_GETARG_TEXT_P(0); + char *formationId = text_to_cstring(formationIdText); + + int32 groupId = PG_GETARG_INT32(1); + + List *nodesGroupList = AutoFailoverNodeGroup(formationId, groupId); + int nodesCount = list_length(nodesGroupList); + + /* + * When there's no nodes registered yet, there's no pg_autoctl process + * that needs the information anyway. Return NULL. + */ + if (nodesCount == 0) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("no nodes found in group %d of formation \"%s\"", + groupId, formationId))); + } + + /* a single node has nothing to synchronize against */ + if (nodesCount == 1) + { + PG_RETURN_TEXT_P(cstring_to_text("")); + } + + AutoFailoverNode *primaryNode = GetPrimaryNodeInGroup(formationId, groupId); + + List *standbyNodesGroupList = AutoFailoverOtherNodesList(primaryNode); + List *candidateNodesList = GroupListCandidates(standbyNodesGroupList); + + if (list_length(candidateNodesList) == 0) + { + PG_RETURN_TEXT_P(cstring_to_text("")); + } + else + { + StringInfo slotNames = makeStringInfo(); + ListCell *nodeCell = NULL; + bool firstNode = true; + + foreach(nodeCell, candidateNodesList) + { + AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); + + appendStringInfo(slotNames, + "%spgautofailover_standby_%lld", + firstNode ? "" : ",", + (long long) node->nodeId); + + if (firstNode) + { + firstNode = false; + } + } + + PG_RETURN_TEXT_P(cstring_to_text(slotNames->data)); + } +} + + /* * testing_lock_formation is a testing-only function that lets an isolation * test hold LockFormation() explicitly, from plain SQL, for as long as its diff --git a/src/monitor/pgautofailover--2.2--2.3.sql b/src/monitor/pgautofailover--2.2--2.3.sql index 871e6b83a..e7de407f6 100644 --- a/src/monitor/pgautofailover--2.2--2.3.sql +++ b/src/monitor/pgautofailover--2.2--2.3.sql @@ -689,3 +689,105 @@ with last_events as ) select * from last_events order by eventtime, eventid; $$; + + +-- +-- Postgres 17+ CDC continuity: synchronized_standby_slots(formation, group) +-- computes the Postgres 17+ "synchronized_standby_slots" setting for a +-- group, the same way synchronous_standby_names is computed, but listing +-- every current failover candidate's physical slot name instead of only +-- the synchronous replication quorum. This is what lets a failover-enabled +-- logical replication slot survive a promotion: it only advances once +-- every node that might get promoted has replayed that far. See +-- docs/cdc.rst for the full picture (issue #1188). +-- + +CREATE FUNCTION pgautofailover.synchronized_standby_slots + ( + IN formation_id text default 'default', + IN group_id int default 0 + ) +RETURNS text LANGUAGE C STRICT +AS 'MODULE_PATHNAME', $$synchronized_standby_slots$$; + +comment on function pgautofailover.synchronized_standby_slots(text, int) + is 'get the Postgres 17+ synchronized_standby_slots setting for a given group, used for CDC/logical slot continuity across failover'; + +grant execute on function + pgautofailover.synchronized_standby_slots(text, int) + to autoctl_node; + +-- Re-create formation_settings() to also report synchronized_standby_slots, +-- next to the existing synchronous_standby_names row. +CREATE OR REPLACE FUNCTION pgautofailover.formation_settings + ( + IN formation_id text default 'default', + OUT context text, + OUT group_id int, + OUT node_id bigint, + OUT nodename text, + OUT setting text, + OUT value text + ) +RETURNS SETOF record LANGUAGE SQL STRICT +AS $$ + with groups(formationid, groupid) as + ( + select formationid, groupid + from pgautofailover.node + where formationid = formation_id + group by formationid, groupid + ) + + -- context: formation, number_sync_standbys + select 'formation' as context, + NULL as group_id, NULL as node_id, formationid as nodename, + 'number_sync_standbys' as setting, + cast(number_sync_standbys as text) as value + from pgautofailover.formation + where formationid = formation_id + +union all + + -- context: primary, one entry per group in the formation + select 'primary', groups.groupid, nodes.node_id, nodes.node_name, + 'synchronous_standby_names', + format('''%s''', + pgautofailover.synchronous_standby_names(formationid, groupid)) + from groups, pgautofailover.get_nodes(formationid, groupid) as nodes + where node_is_primary + +union all + + -- context: primary, synchronized_standby_slots (Postgres 17+ CDC continuity) + select 'primary', groups.groupid, nodes.node_id, nodes.node_name, + 'synchronized_standby_slots', + format('''%s''', + pgautofailover.synchronized_standby_slots(formationid, groupid)) + from groups, pgautofailover.get_nodes(formationid, groupid) as nodes + where node_is_primary + +union all + +( + -- context: node, one entry per node in the formation + select 'node', node.groupid, node.nodeid, node.nodename, + 'replication quorum', cast(node.replicationquorum as text) + from pgautofailover.node as node + where node.formationid = formation_id +order by nodeid +) + +union all + +( + select 'node', node.groupid, node.nodeid, node.nodename, + 'candidate priority', cast(node.candidatepriority as text) + from pgautofailover.node as node + where node.formationid = formation_id +order by nodeid +) +$$; + +comment on function pgautofailover.formation_settings(text) + is 'get the current replication settings a formation'; diff --git a/src/monitor/pgautofailover.sql b/src/monitor/pgautofailover.sql index 8545bf38c..f6f0b64d1 100644 --- a/src/monitor/pgautofailover.sql +++ b/src/monitor/pgautofailover.sql @@ -1228,6 +1228,22 @@ grant execute on function to autoctl_node; +create function pgautofailover.synchronized_standby_slots + ( + IN formation_id text default 'default', + IN group_id int default 0 + ) +returns text language C strict +AS 'MODULE_PATHNAME', $$synchronized_standby_slots$$; + +comment on function pgautofailover.synchronized_standby_slots(text, int) + is 'get the Postgres 17+ synchronized_standby_slots setting for a given group, used for CDC/logical slot continuity across failover'; + +grant execute on function + pgautofailover.synchronized_standby_slots(text, int) + to autoctl_node; + + CREATE FUNCTION pgautofailover.formation_settings ( IN formation_id text default 'default', @@ -1266,6 +1282,16 @@ union all from groups, pgautofailover.get_nodes(formationid, groupid) as nodes where node_is_primary +union all + + -- context: primary, synchronized_standby_slots (Postgres 17+ CDC continuity) + select 'primary', groups.groupid, nodes.node_id, nodes.node_name, + 'synchronized_standby_slots', + format('''%s''', + pgautofailover.synchronized_standby_slots(formationid, groupid)) + from groups, pgautofailover.get_nodes(formationid, groupid) as nodes + where node_is_primary + union all ( diff --git a/src/monitor/regress_schedule b/src/monitor/regress_schedule index 4d2ca7bdb..6d188d8ff 100644 --- a/src/monitor/regress_schedule +++ b/src/monitor/regress_schedule @@ -46,6 +46,7 @@ test: lock_and_fetch_migration test: timeline_fork_detection test: failover_candidate_leaves_secondary test: cluster_init_failover_rule_attribution +test: synchronized_standby_slots test: dummy_update test: drop_extension test: upgrade diff --git a/src/monitor/sql/synchronized_standby_slots.sql b/src/monitor/sql/synchronized_standby_slots.sql new file mode 100644 index 000000000..42a70a7c7 --- /dev/null +++ b/src/monitor/sql/synchronized_standby_slots.sql @@ -0,0 +1,152 @@ +-- Copyright (c) Microsoft Corporation. All rights reserved. +-- Licensed under the PostgreSQL License. +-- +-- Regression test for pgautofailover.synchronized_standby_slots(), the +-- Postgres 17+ CDC-continuity companion to synchronous_standby_names: it +-- lists the physical replication slot names of the group's current +-- failover candidates (AutoFailoverNode.candidatePriority > 0), so that a +-- failover-enabled logical replication slot only advances once every node +-- that might get promoted has replayed that far. +-- +-- Same bootstrap sequence as candidate_count_gate.sql: drive the FSM to +-- primary + secondary + secondary through pgautofailover.node_active(). +-- +-- Unlike candidate_count_gate.sql, register_node()'s result is deliberately +-- NOT printed with "SELECT *": assigned_node_id is a raw sequence value +-- that depends on how many nodes every earlier test in regress_schedule +-- has already registered, which would make this file's expected output +-- fragile to the schedule's exact composition. Only assigned_group_state, +-- which is what this test actually cares about, is printed; node ids are +-- captured silently via \gset for use in node_active() below. + +\x on + +-- ── formation and node registration ───────────────────────────────────────── + +SELECT pgautofailover.create_formation('sss_test', 'pgsql', 'postgres', true, 1); + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_p', 5432, + 'postgres', 'sss_p', 1); + +SELECT nodeid AS np FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_p' \gset + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_s1', 5432, + 'postgres', 'sss_s1', 1); + +SELECT nodeid AS ns1 FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s1' \gset + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_test', 'sss_s2', 5432, + 'postgres', 'sss_s2', 1); + +SELECT nodeid AS ns2 FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s2' \gset + +-- ── bootstrap: drive the FSM to primary + secondary + secondary ───────────── + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'single'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'wait_standby'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'single', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'wait_standby'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'wait_primary', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns1, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'wait_standby'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_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('sss_test', :np, 0, + current_group_role => 'primary', + current_lsn => '0/5000'); + +-- Verify bootstrap: p=primary, s1=secondary, s2=secondary. +SELECT nodename, goalstate, reportedstate, candidatepriority + FROM pgautofailover.node + WHERE formationid = 'sss_test' + ORDER BY nodename; + +-- ── both standbys are candidates: both slot names, in candidatePriority order + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); + +-- ── s2 opts out of the failover quorum: only s1's slot remains ───────────── + +SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s2', 0); + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); + +-- ── no candidates left at all: empty string disables the feature ─────────── + +SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s1', 0); + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); + +-- ── a single-node group has nothing to synchronize against ───────────────── + +SELECT pgautofailover.create_formation('sss_single', 'pgsql', 'postgres', true, 1); + +SELECT assigned_group_state + FROM pgautofailover.register_node('sss_single', 'sss_only', 5432, + 'postgres', 'sss_only', 1); + +SELECT pgautofailover.synchronized_standby_slots('sss_single', 0); + +-- ── an empty group errors out, same as synchronous_standby_names ────────── + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 42); diff --git a/tests/tap/schedule b/tests/tap/schedule index 3b29df16a..b808904f6 100644 --- a/tests/tap/schedule +++ b/tests/tap/schedule @@ -29,6 +29,7 @@ fsm_step_report_advance replication_stall_3dc fast_forward demote_timeout_wait_primary_deadlock +cdc_continuity wait_primary_draining_deadlock timeline_fork_report_lsn_deadlock keeper_fsm_gap_209_wait_maintenance diff --git a/tests/tap/schedules/multi-misc.sch b/tests/tap/schedules/multi-misc.sch index a2439f018..2a2020ae0 100644 --- a/tests/tap/schedules/multi-misc.sch +++ b/tests/tap/schedules/multi-misc.sch @@ -12,3 +12,9 @@ ensure multi_ifdown drop_node_destroy guard_data_loss + +# PG17+ only (the spec itself pins image "pg_auto_failover:pg17", +# regardless of which PGVERSION leg runs this schedule): CDC continuity +# (synchronized_standby_slots, failover-enabled logical slots surviving a +# promotion) and the #1188 stale-failover-slot-after-demote_timeout fix. +cdc_continuity diff --git a/tests/tap/specs/cdc_continuity.pgaf b/tests/tap/specs/cdc_continuity.pgaf new file mode 100644 index 000000000..f8204329d --- /dev/null +++ b/tests/tap/specs/cdc_continuity.pgaf @@ -0,0 +1,155 @@ +# CDC continuity (Postgres 17+): a logical replication slot created with +# failover => true must survive a promotion -- both a clean one (perform +# failover) and one triggered by a primary self-fencing through +# demote_timeout (issue #1188: an interrupted demotion can leave the old +# primary holding stale, non-synced failover slots that permanently block +# the slot-sync worker on the node once it rejoins as a standby). +# +# wal_level must be 'logical' on every node that could ever hold the slot +# (i.e. every node in the group, since any of them can be promoted), so +# setup{} edits postgresql.conf directly and restarts Postgres on each node +# -- there is no pg_autoctl config knob for an arbitrary Postgres GUC like +# wal_level, unlike the pg_autoctl-owned settings under "postgresql.*" +# (see config_get_set.pgaf). +# +# Ordering matters here: node1's restart happens while it is still "single" +# (no synchronous standby attached yet), and node2's restart happens while +# it is merely a secondary. Restarting the *primary* after it already has a +# synchronous standby configured is exactly the risky pattern +# basic_operation.pgaf's own comments warn about ("stopping node1's +# Postgres while it still has a synchronous secondary... reliably triggers +# a real failover"): it was tried that way here first and did reliably +# self-fence node1 into demote_timeout mid-setup. + +cluster { + monitor + image "pg_auto_failover:pg17" + formation { + node1 + node2 + } +} + +setup { + wait until node1 state is single timeout 60s + + exec node1 bash -c "echo \"wal_level = 'logical'\" >> \$PGDATA/postgresql.conf" + stop postgres node1 + start postgres node1 + wait until node1 state is single timeout 60s + + wait until primary, secondary timeout 120s + + exec node2 bash -c "echo \"wal_level = 'logical'\" >> \$PGDATA/postgresql.conf" + stop postgres node2 + start postgres node2 + wait until node2 state is secondary timeout 60s +} + +teardown { + compose down +} + +# +# test_001: create a failover-enabled logical slot on the primary, verify +# the monitor-computed synchronized_standby_slots setting names +# the standby's physical slot, and that the slot gets synced. +# + +step test_001_create_failover_slot { + sql node1 { + SELECT slot_name FROM pg_create_logical_replication_slot( + 'cdc_slot', 'pgoutput', false, false, true); + } + expect { cdc_slot } + + sql node1 { SHOW synchronized_standby_slots; } + expect { pgautofailover_standby_2 } +} + +step test_002_slot_gets_synced_to_standby { + # The slot-sync worker polls periodically; give it a few seconds. + sleep 5s + sql node2 { + SELECT slot_name, failover, synced + FROM pg_replication_slots + WHERE slot_name = 'cdc_slot'; + } + expect { cdc_slot|t|t } +} + +# +# test_003: a clean failover must hand the slot to the new primary still +# usable for logical decoding, with no resync required. Note: +# "synced" stays true forever as a provenance marker (this slot +# originated as a synced copy) -- it is not what governs whether +# the slot can be decoded from, and is not reset on promotion. +# + +step test_003_clean_failover_preserves_slot { + perform failover + wait until node2 state is primary + and node1 state is secondary + timeout 90s + + sql node2 { + SELECT slot_name, failover, synced + FROM pg_replication_slots + WHERE slot_name = 'cdc_slot'; + } + expect { cdc_slot|t|t } +} + +# +# test_004: reproduce issue #1188 -- at this point node2 is primary (from +# test_003) holding the real, promoted copy of cdc_slot, and +# node1 is its standby with a synced copy. node2 self-fences +# through demote_timeout (network partition, same technique as +# demote_timeout_wait_primary_deadlock.pgaf), node1 is promoted, +# and node2 rejoins as a standby still holding its own stale +# (failover AND NOT synced) copy of the slot. Before the fix, +# this permanently blocked node2's slot-sync worker, logging +# "same name slot ... already exists on the standby" forever. +# +# This necessarily re-promotes node1 -- cdc_slot's original +# creator -- a second time, which independently runs into a +# separate, narrower Postgres limitation (see "Limitations" in +# docs/cdc.rst: a node round-tripping back to primary can lose +# its own originally-created slot's data, regardless of this +# fix). That's why test_005 below only asserts the #1188 +# regression itself (no blocking name collision, slot-sync +# worker not stuck) rather than re-asserting data continuity +# already covered by test_002/003 -- doing both at once in a +# 2-node formation would conflate two different limitations. +# A 3-node formation could isolate them fully; left as a +# possible follow-up. +# + +step test_004_primary_self_fences_mid_demotion { + network disconnect node2 + wait until node2 assigned-state = demote_timeout timeout 60s + wait until node1 state is wait_primary timeout 60s + + # Let node2's own keeper notice the partition and stop Postgres + # (network_partition_timeout, default 20s) before reconnecting it. + sleep 30s + network connect node2 +} + +step test_005_old_primary_rejoins_without_the_1188_deadlock { + wait until node2 state is secondary + and node1 state is primary + timeout 90s + + # Give the slot-sync worker's retry loop time to either succeed or get + # stuck -- these are absence checks, so they're only meaningful once + # enough time has passed for the problem to have shown up if it exists. + sleep 20s + + # The slot-sync worker must not be stuck behind node2's own leftover + # failover slot (the exact symptom reported in #1188): neither the + # permanent name-collision error nor an endless "would lose data" retry + # loop (a sign the stale slot cleanup ran too late or not at all). + logs node2 not contains "already exists on the standby" + logs node2 not contains "could not synchronize replication slot" +} From 5b9950e95170f7d6de43df5d07ef34f41d329b42 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 19:24:36 +0000 Subject: [PATCH 02/13] Default wal_level to logical, add --wal-level opt-out, document rollout runbook pg_auto_failover now creates nodes with wal_level=logical by default (required for CDC continuity's failover-enabled replication slots), patched into the existing default GUC settings array at node creation time. A --wal-level replica flag (also settable later via the keeper config file) opts back out for deployments that don't need logical decoding. docs/cdc.rst gains the manual runbook for flipping wal_level on an already-running cluster via maintenance mode, and docs/orchestrated-operations.rst explores a generalized monitor/client primitive for driving that kind of whole-cluster, multi-step change as a single declared, resumable, cancellable operation. cdc_continuity.pgaf is simplified to rely on the new default instead of manually toggling wal_level in its setup block. --- docs/cdc.rst | 95 ++++++++++- docs/index.rst | 1 + docs/operations.rst | 4 + docs/orchestrated-operations.rst | 231 +++++++++++++++++++++++++++ src/bin/common/pgsetup.h | 1 + src/bin/pg_autoctl/cli_common.c | 16 ++ src/bin/pg_autoctl/cli_create_node.c | 6 +- src/bin/pg_autoctl/keeper_config.c | 5 + src/bin/pg_autoctl/primary_standby.c | 50 +++++- tests/tap/specs/cdc_continuity.pgaf | 40 ++--- 10 files changed, 414 insertions(+), 35 deletions(-) create mode 100644 docs/orchestrated-operations.rst diff --git a/docs/cdc.rst b/docs/cdc.rst index 5e9ada0d8..abc509b9e 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -79,12 +79,17 @@ replication slots: that remains entirely your CDC tool's responsibility, exactly as it would be on a Postgres cluster not managed by pg_auto_failover. Concretely: - 1. Set ``wal_level = logical`` yourself. pg_auto_failover does not force - this setting on 17+ nodes, since it has real costs (extra WAL volume, - catalog ``xmin`` retention) that only make sense if you are actually - using logical replication. You can set it through ``pg_autoctl config - set``, or directly in ``postgresql.conf``; either way it requires a - restart, same as on any Postgres instance. + 1. ``wal_level`` is ``logical`` by default on every node pg_auto_failover + creates (see :ref:`wal_level_default` below) -- there is nothing to do + here for a new cluster. If you're turning CDC on for an + *already-running* cluster that was created with the default, or with + ``--wal-level replica``, see :ref:`wal_level_rollout` for how to change + it safely: ``wal_level`` is ``PGC_POSTMASTER`` (Postgres won't let you + change it without a full server restart), and **every** node needs it, + not just the current primary -- any node could be promoted later, and a + node's own ``wal_level`` governs whether it's allowed to decode + logically at all, independently of what the primary that originally + wrote the WAL had configured. 2. Create your logical replication slot(s) with ``failover => true``, for example:: @@ -103,6 +108,84 @@ pg_auto_failover. Concretely: redirects regular clients transparently; the same connection logic applies to your CDC consumer's connection to the (new) primary. +.. _wal_level_default: + +Defaults and opting out +------------------------ + +Every node pg_auto_failover creates (``pg_autoctl create postgres``, +``create monitor``, ``create coordinator``, ``create worker``) sets +``wal_level = logical`` by default -- the cost (extra WAL volume, catalog +``xmin`` retention pressure) is paid whether or not you ever create a +slot, so if you know you'll never need CDC on a given cluster, you can +opt back out at creation time:: + + pg_autoctl create postgres --wal-level replica ... + +The flag only takes effect at node creation, before Postgres's first +start: ``wal_level`` can't be changed by editing the config and reloading +on a running node (see :ref:`wal_level_rollout` for that case). There is +no supported way to go the other direction (``replica`` back up to +``logical``) except through that same rollout procedure, applied to a +cluster that opted out at creation time. + +.. _wal_level_rollout: + +Turning on ``wal_level = logical`` on an already-running cluster +------------------------------------------------------------------- + +Because ``wal_level`` is ``PGC_POSTMASTER``, there's no way around a full +Postgres restart on every node -- but pg_auto_failover already has the +primitives to do that fleet-wide with zero *unplanned* downtime, using +exactly the same maintenance-mode mechanism documented in +:ref:`upgrading_from_previous_versions`, which solves the identical +problem -- "a change needs a restart on every node, without triggering a +spurious failover" -- for a different cause (a pg_auto_failover version +upgrade rather than a GUC change). The one planned switchover below costs +the same single, short write-unavailability window as any manual +switchover with this tool, nothing more. + +For a formation with one primary and one or more standbys: + + 1. For each **standby**, one at a time: + + a. ``pg_autoctl enable maintenance`` -- safe: the monitor switches the + primary to asynchronous replication and the standby is no longer a + promotion candidate while in this state. + b. Apply the change, either directly in ``postgresql.conf`` or via + ``ALTER SYSTEM SET wal_level = 'logical';`` (both are safe: + pg_auto_failover's own managed settings live in a separate included + file, so this never collides with anything it writes). + c. Restart Postgres on that node (``pg_ctl restart``, your init + system, or however you'd restart it for a kernel/Postgres point + release). + d. ``pg_autoctl disable maintenance`` to let it rejoin. + + 2. Once **every standby** has been upgraded, promote one of them and + demote the old primary in one controlled step:: + + pg_autoctl enable maintenance --allow-failover + + This runs the same promotion path as a normal switchover and leaves + the old primary in ``maintenance`` state as a standby -- this is the + one and only planned write-unavailability window in the whole + procedure. + + 3. Apply steps 1b-1d to the now-demoted former primary, then + ``pg_autoctl disable maintenance`` to bring it back as a regular + standby. + +At the end, every node has ``wal_level = logical`` and the formation is +back to its original primary/standby layout (or not, if you'd rather +leave the promoted standby as the new primary -- nothing above requires +failing back). + +This is a manual, multi-command procedure today. See +:ref:`orchestrated_operations` for a design exploration of a generalized +primitive that could drive exactly this sequence as a single, checkable, +resumable operation -- independently of CDC, and reusable for other +whole-cluster changes. + How this compares to other HA tools ------------------------------------ diff --git a/docs/index.rst b/docs/index.rst index edb2eb624..1943b2cf3 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -56,6 +56,7 @@ __ https://github.com/hapostgres/pg_auto_failover failover-state-machine fault-tolerance cdc + orchestrated-operations security .. toctree:: diff --git a/docs/operations.rst b/docs/operations.rst index bf79f60ab..9251c467f 100644 --- a/docs/operations.rst +++ b/docs/operations.rst @@ -1,3 +1,5 @@ +.. _operations: + Operating pg_auto_failover ========================== @@ -194,6 +196,8 @@ monitor. And when the upgrade is done we can use ``pg_autoctl show state`` on the monitor to see that eveything is as expected. +.. _upgrading_from_previous_versions: + Upgrading from previous pg_auto_failover versions ------------------------------------------------- diff --git a/docs/orchestrated-operations.rst b/docs/orchestrated-operations.rst new file mode 100644 index 000000000..404cd6102 --- /dev/null +++ b/docs/orchestrated-operations.rst @@ -0,0 +1,231 @@ +.. _orchestrated_operations: + +Orchestrated Whole-Cluster Operations (design exploration) +============================================================= + +This page explores what it would take to let pg_auto_failover drive a +multi-step, multi-node change -- such as the :ref:`wal_level_rollout` +runbook -- as a single declared operation: started once, resumable, +checkable, and cancellable, rather than a sequence of commands an operator +runs and tracks by hand. It's a design exploration, not a committed +feature: nothing described here is implemented. It came out of looking for +a way to automate the ``wal_level`` rollout, but the problem is general +enough to be worth solving once, for every future case that needs it +(a rolling pg_autoctl/Postgres minor-version upgrade across a fleet, a +formation-wide SSL certificate rotation, re-balancing candidate priorities +-- anything that is "do a safe sequence of steps across several nodes, in +an order that matters"). + +Why this fits pg_auto_failover's existing architecture +-------------------------------------------------------- + +pg_auto_failover is **pull-based**, not push-based, and that invariant is +worth preserving: the monitor never opens a connection *to* a data node, +runs commands there, or otherwise reaches out. Every ``pg_autoctl`` keeper +process is the one initiating contact, on its own schedule, asking the +monitor "what's my goal state?" (``node_active()``, see +``src/monitor/node_active_protocol.c``) and reporting back once it's +reached it. The monitor's entire job is to *decide* goal states from a +declarative dispatch table (``MonitorFSM[]`` in +``src/monitor/group_state_machine.c``) and record intent; keepers are the +only thing that ever executes anything against a live Postgres. + +A generalized operation has to respect that split. The step in the +``wal_level`` runbook that doesn't exist today -- "edit postgresql.conf, +restart Postgres" -- can only ever be performed by the keeper that already +owns that node's Postgres process (``service_postgres.c``, +``primary_standby.c``), never by the monitor reaching out to do it. So the +right shape is: the monitor stores *intent* ("this node should apply GUC +X=Y, requires a restart"); each keeper discovers its own pending intent +the same way it already discovers its goal state, and applies it +autonomously, in-process, using machinery it already has (it already +stops/edits/restarts Postgres routinely -- see ``fsm_transition.c``'s +maintenance and promotion paths, and ``postgres_add_default_settings()`` +in ``primary_standby.c``, which is the same function the ``wal_level`` +default added in this PR goes through). + +What's missing is the thing *above* individual keeper intent: a way to +express "do this to every node in the group, standbys first, in a +specific order, and track where we are" -- a level pg_auto_failover +doesn't have today, sitting between the formation/group model +(``pgautofailover.formation`` / ``pgautofailover.node``) and the per-node +FSM. + +Proposed shape +--------------- + +**Monitor schema** (new tables, alongside ``pgautofailover.node``): + +.. code-block:: sql + + CREATE TYPE pgautofailover.operation_status AS ENUM + ( + 'pending', 'running', 'blocked', 'completed', 'failed', 'cancelled' + ); + + CREATE TABLE pgautofailover.operation + ( + operationid bigserial PRIMARY KEY, + formationid text NOT NULL REFERENCES pgautofailover.formation, + groupid int, -- NULL = every group in the formation + kind text NOT NULL, -- 'guc_rollout', future kinds... + parameters jsonb NOT NULL, -- e.g. {"guc": "wal_level", "value": "logical"} + status pgautofailover.operation_status NOT NULL DEFAULT 'pending', + allow_failover bool NOT NULL DEFAULT false, + created_by text, + created_at timestamptz NOT NULL DEFAULT now(), + started_at timestamptz, + completed_at timestamptz, + error_message text + ); + + CREATE TABLE pgautofailover.operation_step + ( + operationid bigint NOT NULL REFERENCES pgautofailover.operation, + stepno int NOT NULL, + nodeid bigint NOT NULL REFERENCES pgautofailover.node, + action text NOT NULL, -- 'apply_pending_guc', 'promote', ... + status pgautofailover.operation_status NOT NULL DEFAULT 'pending', + started_at timestamptz, + completed_at timestamptz, + error_message text, + PRIMARY KEY (operationid, stepno) + ); + +A per-node **pending change** column on ``pgautofailover.node`` is the +bridge to the keeper side -- the only thing a keeper needs to notice on +its own next ``node_active()`` round-trip:: + + ALTER TABLE pgautofailover.node + ADD COLUMN pending_guc_changes jsonb; + +**Monitor API** (new SQL functions, mirroring ``perform_failover()`` / +``start_maintenance()``'s existing style in +``src/monitor/node_active_protocol.c`` / ``formation_metadata.c``): + +- ``pgautofailover.start_operation(formation_id, kind, parameters jsonb, + allow_failover bool default false) RETURNS bigint`` -- validates + preconditions for the given ``kind`` (e.g. for ``guc_rollout``: the GUC + name/value pair is well-formed, the formation/group exists), computes + the step plan (standbys in arbitrary order, then -- if the primary also + needs the change -- a promotion step requiring ``allow_failover``, then + the demoted former primary), inserts the ``operation`` and + ``operation_step`` rows, and sets ``pending_guc_changes`` on the first + step's target node(s). Takes the same ``LockFormation()``/ + ``LockNodeGroup()`` discipline already used by ``perform_failover()`` and + ``remove_node()``, since it mutates the same per-group state those do. + +- ``pgautofailover.get_operation(operation_id)`` / ``list_operations(formation_id)`` + -- read-only status, current step, per-step history. This is what + ``pg_autoctl show operation `` / ``show operations`` would query. + +- ``pgautofailover.advance_operations()`` -- the periodic sweep: for every + ``running`` operation, check whether the current step's target node(s) + have cleared their ``pending_guc_changes`` (keeper-reported, see below) + or reached the expected state (e.g. ``maintenance``, or back to + ``secondary``/``primary`` after a promotion step); if so, mark the step + ``completed`` and set ``pending_guc_changes`` on the *next* step's + node(s); if a step has been stuck past a configurable timeout, mark it + ``blocked`` instead of silently waiting forever. Called the same way + ``ProceedGroupState()`` already gets invoked opportunistically on every + ``node_active()`` call (no new background worker needed, in contrast to + ``health_check_worker.c``'s separate bgworker approach -- an operation + only needs to progress when *some* node next reports in, which already + happens on pg_auto_failover's existing health-check cadence). + +- ``pgautofailover.unblock_operation(operation_id, ...)`` -- operator + override for a ``blocked`` step: either retry it, or mark it manually + resolved (e.g. "I restarted Postgres on that node myself, move on") and + advance to the next step. + +- ``pgautofailover.cancel_operation(operation_id)`` -- clears + ``pending_guc_changes`` on any node whose step hasn't completed yet, and + marks the operation ``cancelled``. Steps already applied are not + automatically rolled back (see *Rollback* below). + +**Keeper side** (``src/bin/pg_autoctl``): the keeper's existing per-tick +loop (``keeper_ensure_current_state()`` in ``keeper.c``, the same place +that already drives replication-slot maintenance and settings +application) gains one more check: if ``pending_guc_changes`` is non-empty +for this node, + +- on a **standby**: apply it the same way ``postgres_add_default_settings()`` + already writes GUCs today, restart Postgres (the same stop/start + machinery ``service_postgres.c`` already uses for its own "ensure" + restarts), report success by clearing its own ``pending_guc_changes`` + via ``node_active()``'s existing report payload (one more field, + alongside ``reportedLSN``/``reportedTLI``/etc. in + ``node_active_protocol.c``); +- on the **primary**: refuse and report a benign "not yet" status *unless* + the operation's ``allow_failover`` was set, in which case it requests a + promotion the same way ``pg_autoctl enable maintenance + --allow-failover`` already does today (reusing + ``perform_failover()``), and only applies its own pending change once + demoted. + +This is deliberately the *same* sequence as the manual runbook in +:ref:`wal_level_rollout` -- the design doesn't invent new mechanics, it +automates the existing ones end to end. + +**Client CLI surface** (new subcommands, following the existing +``pg_autoctl perform failover`` / ``enable maintenance`` naming): + +.. code-block:: bash + + pg_autoctl perform rolling-change --formation default \ + --guc wal_level=logical [--allow-failover] + + pg_autoctl show operations [--formation default] + pg_autoctl show operation + pg_autoctl operation unblock [--step ] + pg_autoctl operation cancel + +``perform rolling-change`` is a thin wrapper around +``start_operation()``; ``show operations``/``show operation`` wrap +``get_operation()``/``list_operations()``; the two ``operation`` +subcommands wrap ``unblock_operation()``/``cancel_operation()``. None of +these need the client to stay connected or keep polling itself -- all the +state lives on the monitor, same as every other pg_auto_failover +operation, so the CLI can be re-run from anywhere, at any time, to check +or act on an in-flight operation. + +Open questions +--------------- + +- **Rollback semantics are necessarily per-``kind``, not generic.** + "Cancel" can always stop handing out *new* pending changes, but a step + already applied (Postgres already restarted with the new ``wal_level``) + can only be undone by another forward-moving change (queue + ``wal_level=replica`` as a new pending change on the nodes that already + flipped) -- there is no actual "undo a restart". Each ``kind`` would + need to define both its forward step plan and its compensating one. + +- **Concurrent operations on the same formation/group.** ``start_operation()`` + needs to refuse (or queue) a second operation against a group that + already has one ``running``/``blocked`` -- otherwise two operations + could both decide it's safe to promote at the same time. The existing + ``LockNodeGroup()`` discipline likely extends to this directly, since + it's the same lock ``perform_failover()`` already takes for exactly this + class of race. + +- **Permissions.** ``start_operation()``/``unblock_operation()``/ + ``cancel_operation()`` are operator actions, not something a keeper + should ever call autonomously -- same ``autoctl`` vs. ``autoctl_node`` + role split already used for ``start_maintenance()`` vs. the keeper's own + calls (see :ref:`operations`'s note on the ``autoctl_node`` role being + "not allowed to perform maintenance operations"). + +- **Interaction with manual maintenance mode.** An operator running + ``pg_autoctl enable maintenance`` by hand on a node that's also mid-step + in an orchestrated operation needs a defined precedence -- probably: + manual maintenance always wins, and the operation's own step simply + waits (or is marked ``blocked`` once its timeout elapses), so an + operator always retains the ability to intervene without the two + mechanisms fighting each other. + +- **Scope of "whole-cluster."** The schema above is written per-formation/ + per-group, matching how pg_auto_failover already scopes everything else + (``synchronous_standby_names``, ``synchronized_standby_slots``, + failover). A true multi-formation operation (e.g. a Citus cluster's + coordinator and every worker group) would need an operation to *contain* + other operations, one per group -- not explored further here. diff --git a/src/bin/common/pgsetup.h b/src/bin/common/pgsetup.h index 55afba8e3..ae1db488c 100644 --- a/src/bin/common/pgsetup.h +++ b/src/bin/common/pgsetup.h @@ -233,6 +233,7 @@ typedef struct pg_setup NodeReplicationSettings settings; /* node replication settings */ SSLOptions ssl; /* ssl options */ char citusClusterName[NAMEDATALEN]; /* citus.cluster_name */ + char walLevel[NAMEDATALEN]; /* wal_level, defaults to "logical" */ } PostgresSetup; #define IS_EMPTY_STRING_BUFFER(strbuf) (strbuf[0] == '\0') diff --git a/src/bin/pg_autoctl/cli_common.c b/src/bin/pg_autoctl/cli_common.c index 709504b7c..80f1634f3 100644 --- a/src/bin/pg_autoctl/cli_common.c +++ b/src/bin/pg_autoctl/cli_common.c @@ -241,6 +241,22 @@ cli_common_keeper_getopts(int argc, char **argv, break; } + case 'K': + { + /* { "wal-level", required_argument, NULL, 'K' }, */ + if (strcmp(optarg, "logical") != 0 && strcmp(optarg, "replica") != 0) + { + errors++; + log_error("Invalid value for --wal-level: \"%s\", " + "expected \"logical\" (the default) or \"replica\"", + optarg); + } + + strlcpy(LocalOptionConfig.pgSetup.walLevel, optarg, NAMEDATALEN); + log_trace("--wal-level %s", LocalOptionConfig.pgSetup.walLevel); + break; + } + case 'S': { /* { "skip-pg-hba", required_argument, NULL, 'S' }, */ diff --git a/src/bin/pg_autoctl/cli_create_node.c b/src/bin/pg_autoctl/cli_create_node.c index 39b3f4aab..c2a8de303 100644 --- a/src/bin/pg_autoctl/cli_create_node.c +++ b/src/bin/pg_autoctl/cli_create_node.c @@ -102,7 +102,8 @@ CommandLine create_postgres_command = KEEPER_CLI_SSL_OPTIONS " --candidate-priority priority of the node to be promoted to become primary\n" " --replication-quorum true if node participates in write quorum\n" - " --maximum-backup-rate maximum transfer rate of data transferred from the server during initial sync\n", + " --maximum-backup-rate maximum transfer rate of data transferred from the server during initial sync\n" + " --wal-level 'logical' (the default, needed for CDC) or 'replica'\n", cli_create_postgres_getopts, cli_create_postgres); @@ -344,6 +345,7 @@ cli_create_postgres_getopts(int argc, char **argv) { "replication-quorum", required_argument, NULL, 'r' }, { "maximum-backup-rate", required_argument, NULL, 'R' }, { "region", required_argument, NULL, 'G' }, + { "wal-level", required_argument, NULL, 'K' }, { "run", no_argument, NULL, 'x' }, { "no-ssl", no_argument, NULL, 'N' }, { "ssl-self-signed", no_argument, NULL, 's' }, @@ -357,7 +359,7 @@ cli_create_postgres_getopts(int argc, char **argv) int optind = cli_create_node_getopts(argc, argv, long_options, - "C:D:H:p:l:U:A:SLd:a:n:f:m:MI:W:w:RGVvqhP:r:xsN", + "C:D:H:p:l:U:A:SLd:a:n:f:m:MI:W:w:RGK:VvqhP:r:xsN", &options); /* publish our option parsing in the global variable */ diff --git a/src/bin/pg_autoctl/keeper_config.c b/src/bin/pg_autoctl/keeper_config.c index 7621ff530..918f1390e 100644 --- a/src/bin/pg_autoctl/keeper_config.c +++ b/src/bin/pg_autoctl/keeper_config.c @@ -103,6 +103,10 @@ make_strbuf_option("postgresql", "hba_level", NULL, \ false, MAXPGPATH, config->pgSetup.hbaLevelStr) +#define OPTION_POSTGRESQL_WAL_LEVEL(config) \ + make_strbuf_option("postgresql", "wal_level", "wal-level", \ + false, NAMEDATALEN, config->pgSetup.walLevel) + #define OPTION_SSL_ACTIVE(config) \ make_int_option_default("ssl", "active", NULL, \ false, &(config->pgSetup.ssl.active), 0) @@ -237,6 +241,7 @@ OPTION_POSTGRESQL_LISTEN_ADDRESSES(config), \ OPTION_POSTGRESQL_AUTH_METHOD(config), \ OPTION_POSTGRESQL_HBA_LEVEL(config), \ + OPTION_POSTGRESQL_WAL_LEVEL(config), \ OPTION_SSL_ACTIVE(config), \ OPTION_SSL_MODE(config), \ OPTION_SSL_CA_FILE(config), \ diff --git a/src/bin/pg_autoctl/primary_standby.c b/src/bin/pg_autoctl/primary_standby.c index 409b9a34d..c615cb388 100644 --- a/src/bin/pg_autoctl/primary_standby.c +++ b/src/bin/pg_autoctl/primary_standby.c @@ -47,7 +47,10 @@ static void local_postgres_update_pg_failures_tracking(LocalPostgresServer *post { "port", "5432" }, \ { "max_wal_senders", "12" }, \ { "max_replication_slots", "12" }, \ - { "wal_level", "'replica'" }, \ + { "wal_level", "'replica'" }, /* postgres_add_default_settings() patches \ + * this entry to 'logical' by default, or \ + * --wal-level replica's value, for \ + * non-Citus nodes -- see there. */ \ { "wal_log_hints", "on" }, \ { "wal_sender_timeout", "'30s'" }, \ { "hot_standby_feedback", "on" }, \ @@ -901,6 +904,8 @@ postgres_add_default_settings(LocalPostgresServer *postgres, PostgresSetup *pgSetup = &(postgres->postgresSetup); char configFilePath[MAXPGPATH]; GUC *default_settings = NULL; + GUC patchedSettings[32] = { 0 }; + char walLevelValue[NAMEDATALEN + 2] = { 0 }; log_trace("postgres_add_default_settings (%s) [%d]", nodeKindToString(postgres->pgKind), @@ -966,6 +971,49 @@ postgres_add_default_settings(LocalPostgresServer *postgres, } } + /* + * wal_level defaults to 'logical' on every plain (non-Citus) node: + * CDC continuity (see docs/cdc.rst) needs it on any node that might + * ever become primary, which in pg_auto_failover is every node in the + * group. Citus nodes already force 'logical' unconditionally above, + * for Citus's own unrelated needs, so they're left out of this + * override. --wal-level replica at node creation time (pgSetup->walLevel) + * opts back out, for clusters that know they'll never use CDC and + * would rather not pay wal_level=logical's WAL-volume and catalog + * xmin-retention cost. + * + * The static default_settings arrays above are copied into a local, + * patched array here rather than hardcoding the value into each of + * them, so there's exactly one place that decides the actual value + * written to postgresql.conf. + */ + if (!IS_CITUS_INSTANCE_KIND(postgres->pgKind)) + { + const char *requestedWalLevel = + IS_EMPTY_STRING_BUFFER(pgSetup->walLevel) + ? "logical" + : pgSetup->walLevel; + int i = 0; + int maxEntries = (int) (sizeof(patchedSettings) / + sizeof(patchedSettings[0])); + + sformat(walLevelValue, sizeof(walLevelValue), "'%s'", requestedWalLevel); + + for (i = 0; default_settings[i].name != NULL && i < maxEntries - 1; i++) + { + patchedSettings[i] = default_settings[i]; + + if (strcmp(default_settings[i].name, "wal_level") == 0) + { + patchedSettings[i].value = walLevelValue; + } + } + patchedSettings[i].name = NULL; + patchedSettings[i].value = NULL; + + default_settings = patchedSettings; + } + if (!pg_add_auto_failover_default_settings(pgSetup, hostname, configFilePath, diff --git a/tests/tap/specs/cdc_continuity.pgaf b/tests/tap/specs/cdc_continuity.pgaf index f8204329d..79f890794 100644 --- a/tests/tap/specs/cdc_continuity.pgaf +++ b/tests/tap/specs/cdc_continuity.pgaf @@ -5,21 +5,21 @@ # primary holding stale, non-synced failover slots that permanently block # the slot-sync worker on the node once it rejoins as a standby). # -# wal_level must be 'logical' on every node that could ever hold the slot -# (i.e. every node in the group, since any of them can be promoted), so -# setup{} edits postgresql.conf directly and restarts Postgres on each node -# -- there is no pg_autoctl config knob for an arbitrary Postgres GUC like -# wal_level, unlike the pg_autoctl-owned settings under "postgresql.*" -# (see config_get_set.pgaf). +# wal_level defaults to 'logical' on every node pg_auto_failover creates +# (see docs/cdc.rst), so there is nothing to set up here beyond the +# default: test_001 below creating a failover-enabled logical slot right +# after setup{} is itself the regression guard that the default actually +# took effect on both nodes (it would fail outright with a plain Postgres +# error otherwise, same as it would on wal_level='replica'). # -# Ordering matters here: node1's restart happens while it is still "single" -# (no synchronous standby attached yet), and node2's restart happens while -# it is merely a secondary. Restarting the *primary* after it already has a -# synchronous standby configured is exactly the risky pattern -# basic_operation.pgaf's own comments warn about ("stopping node1's -# Postgres while it still has a synchronous secondary... reliably triggers -# a real failover"): it was tried that way here first and did reliably -# self-fence node1 into demote_timeout mid-setup. +# Earlier revisions of this spec had to flip wal_level on after the fact, +# and getting that ordering wrong (restarting the *primary* after it +# already had a synchronous standby attached) reliably self-fenced it into +# demote_timeout mid-setup -- exactly the risky pattern +# basic_operation.pgaf's own comments warn about. See +# docs/cdc.rst's wal_level rollout runbook for that procedure, still +# needed for a cluster created before this default, or with +# --wal-level replica. cluster { monitor @@ -31,19 +31,7 @@ cluster { } setup { - wait until node1 state is single timeout 60s - - exec node1 bash -c "echo \"wal_level = 'logical'\" >> \$PGDATA/postgresql.conf" - stop postgres node1 - start postgres node1 - wait until node1 state is single timeout 60s - wait until primary, secondary timeout 120s - - exec node2 bash -c "echo \"wal_level = 'logical'\" >> \$PGDATA/postgresql.conf" - stop postgres node2 - start postgres node2 - wait until node2 state is secondary timeout 60s } teardown { From 59e792a1d66478dae10105461e55408ea7ff3bbf Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:11:52 +0000 Subject: [PATCH 03/13] Fix cdc_continuity CI image pull, drop orchestrated-operations design doc The spec hardcoded image "pg_auto_failover:pg17", a tag that only ever existed on a local Docker build used for manual validation. CI builds and loads pgaf:run-pg and passes it via PGAF_IMAGE, which an explicit image "..." in the spec overrides -- so CI tried to pull a nonexistent "pg_auto_failover:pg17" from a registry and failed. Dropping the explicit image line falls back to PGAF_IMAGE, same as every other spec in the suite. Also removes the orchestrated-operations design exploration from the user-facing docs (toctree entry and the cdc.rst cross-reference) and deletes the page -- not ready to present as part of this feature's documentation. --- docs/cdc.rst | 6 - docs/index.rst | 1 - docs/orchestrated-operations.rst | 231 ---------------------------- tests/tap/schedules/multi-misc.sch | 8 +- tests/tap/specs/cdc_continuity.pgaf | 1 - 5 files changed, 4 insertions(+), 243 deletions(-) delete mode 100644 docs/orchestrated-operations.rst diff --git a/docs/cdc.rst b/docs/cdc.rst index abc509b9e..3905c846c 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -180,12 +180,6 @@ back to its original primary/standby layout (or not, if you'd rather leave the promoted standby as the new primary -- nothing above requires failing back). -This is a manual, multi-command procedure today. See -:ref:`orchestrated_operations` for a design exploration of a generalized -primitive that could drive exactly this sequence as a single, checkable, -resumable operation -- independently of CDC, and reusable for other -whole-cluster changes. - How this compares to other HA tools ------------------------------------ diff --git a/docs/index.rst b/docs/index.rst index 1943b2cf3..edb2eb624 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -56,7 +56,6 @@ __ https://github.com/hapostgres/pg_auto_failover failover-state-machine fault-tolerance cdc - orchestrated-operations security .. toctree:: diff --git a/docs/orchestrated-operations.rst b/docs/orchestrated-operations.rst deleted file mode 100644 index 404cd6102..000000000 --- a/docs/orchestrated-operations.rst +++ /dev/null @@ -1,231 +0,0 @@ -.. _orchestrated_operations: - -Orchestrated Whole-Cluster Operations (design exploration) -============================================================= - -This page explores what it would take to let pg_auto_failover drive a -multi-step, multi-node change -- such as the :ref:`wal_level_rollout` -runbook -- as a single declared operation: started once, resumable, -checkable, and cancellable, rather than a sequence of commands an operator -runs and tracks by hand. It's a design exploration, not a committed -feature: nothing described here is implemented. It came out of looking for -a way to automate the ``wal_level`` rollout, but the problem is general -enough to be worth solving once, for every future case that needs it -(a rolling pg_autoctl/Postgres minor-version upgrade across a fleet, a -formation-wide SSL certificate rotation, re-balancing candidate priorities --- anything that is "do a safe sequence of steps across several nodes, in -an order that matters"). - -Why this fits pg_auto_failover's existing architecture --------------------------------------------------------- - -pg_auto_failover is **pull-based**, not push-based, and that invariant is -worth preserving: the monitor never opens a connection *to* a data node, -runs commands there, or otherwise reaches out. Every ``pg_autoctl`` keeper -process is the one initiating contact, on its own schedule, asking the -monitor "what's my goal state?" (``node_active()``, see -``src/monitor/node_active_protocol.c``) and reporting back once it's -reached it. The monitor's entire job is to *decide* goal states from a -declarative dispatch table (``MonitorFSM[]`` in -``src/monitor/group_state_machine.c``) and record intent; keepers are the -only thing that ever executes anything against a live Postgres. - -A generalized operation has to respect that split. The step in the -``wal_level`` runbook that doesn't exist today -- "edit postgresql.conf, -restart Postgres" -- can only ever be performed by the keeper that already -owns that node's Postgres process (``service_postgres.c``, -``primary_standby.c``), never by the monitor reaching out to do it. So the -right shape is: the monitor stores *intent* ("this node should apply GUC -X=Y, requires a restart"); each keeper discovers its own pending intent -the same way it already discovers its goal state, and applies it -autonomously, in-process, using machinery it already has (it already -stops/edits/restarts Postgres routinely -- see ``fsm_transition.c``'s -maintenance and promotion paths, and ``postgres_add_default_settings()`` -in ``primary_standby.c``, which is the same function the ``wal_level`` -default added in this PR goes through). - -What's missing is the thing *above* individual keeper intent: a way to -express "do this to every node in the group, standbys first, in a -specific order, and track where we are" -- a level pg_auto_failover -doesn't have today, sitting between the formation/group model -(``pgautofailover.formation`` / ``pgautofailover.node``) and the per-node -FSM. - -Proposed shape ---------------- - -**Monitor schema** (new tables, alongside ``pgautofailover.node``): - -.. code-block:: sql - - CREATE TYPE pgautofailover.operation_status AS ENUM - ( - 'pending', 'running', 'blocked', 'completed', 'failed', 'cancelled' - ); - - CREATE TABLE pgautofailover.operation - ( - operationid bigserial PRIMARY KEY, - formationid text NOT NULL REFERENCES pgautofailover.formation, - groupid int, -- NULL = every group in the formation - kind text NOT NULL, -- 'guc_rollout', future kinds... - parameters jsonb NOT NULL, -- e.g. {"guc": "wal_level", "value": "logical"} - status pgautofailover.operation_status NOT NULL DEFAULT 'pending', - allow_failover bool NOT NULL DEFAULT false, - created_by text, - created_at timestamptz NOT NULL DEFAULT now(), - started_at timestamptz, - completed_at timestamptz, - error_message text - ); - - CREATE TABLE pgautofailover.operation_step - ( - operationid bigint NOT NULL REFERENCES pgautofailover.operation, - stepno int NOT NULL, - nodeid bigint NOT NULL REFERENCES pgautofailover.node, - action text NOT NULL, -- 'apply_pending_guc', 'promote', ... - status pgautofailover.operation_status NOT NULL DEFAULT 'pending', - started_at timestamptz, - completed_at timestamptz, - error_message text, - PRIMARY KEY (operationid, stepno) - ); - -A per-node **pending change** column on ``pgautofailover.node`` is the -bridge to the keeper side -- the only thing a keeper needs to notice on -its own next ``node_active()`` round-trip:: - - ALTER TABLE pgautofailover.node - ADD COLUMN pending_guc_changes jsonb; - -**Monitor API** (new SQL functions, mirroring ``perform_failover()`` / -``start_maintenance()``'s existing style in -``src/monitor/node_active_protocol.c`` / ``formation_metadata.c``): - -- ``pgautofailover.start_operation(formation_id, kind, parameters jsonb, - allow_failover bool default false) RETURNS bigint`` -- validates - preconditions for the given ``kind`` (e.g. for ``guc_rollout``: the GUC - name/value pair is well-formed, the formation/group exists), computes - the step plan (standbys in arbitrary order, then -- if the primary also - needs the change -- a promotion step requiring ``allow_failover``, then - the demoted former primary), inserts the ``operation`` and - ``operation_step`` rows, and sets ``pending_guc_changes`` on the first - step's target node(s). Takes the same ``LockFormation()``/ - ``LockNodeGroup()`` discipline already used by ``perform_failover()`` and - ``remove_node()``, since it mutates the same per-group state those do. - -- ``pgautofailover.get_operation(operation_id)`` / ``list_operations(formation_id)`` - -- read-only status, current step, per-step history. This is what - ``pg_autoctl show operation `` / ``show operations`` would query. - -- ``pgautofailover.advance_operations()`` -- the periodic sweep: for every - ``running`` operation, check whether the current step's target node(s) - have cleared their ``pending_guc_changes`` (keeper-reported, see below) - or reached the expected state (e.g. ``maintenance``, or back to - ``secondary``/``primary`` after a promotion step); if so, mark the step - ``completed`` and set ``pending_guc_changes`` on the *next* step's - node(s); if a step has been stuck past a configurable timeout, mark it - ``blocked`` instead of silently waiting forever. Called the same way - ``ProceedGroupState()`` already gets invoked opportunistically on every - ``node_active()`` call (no new background worker needed, in contrast to - ``health_check_worker.c``'s separate bgworker approach -- an operation - only needs to progress when *some* node next reports in, which already - happens on pg_auto_failover's existing health-check cadence). - -- ``pgautofailover.unblock_operation(operation_id, ...)`` -- operator - override for a ``blocked`` step: either retry it, or mark it manually - resolved (e.g. "I restarted Postgres on that node myself, move on") and - advance to the next step. - -- ``pgautofailover.cancel_operation(operation_id)`` -- clears - ``pending_guc_changes`` on any node whose step hasn't completed yet, and - marks the operation ``cancelled``. Steps already applied are not - automatically rolled back (see *Rollback* below). - -**Keeper side** (``src/bin/pg_autoctl``): the keeper's existing per-tick -loop (``keeper_ensure_current_state()`` in ``keeper.c``, the same place -that already drives replication-slot maintenance and settings -application) gains one more check: if ``pending_guc_changes`` is non-empty -for this node, - -- on a **standby**: apply it the same way ``postgres_add_default_settings()`` - already writes GUCs today, restart Postgres (the same stop/start - machinery ``service_postgres.c`` already uses for its own "ensure" - restarts), report success by clearing its own ``pending_guc_changes`` - via ``node_active()``'s existing report payload (one more field, - alongside ``reportedLSN``/``reportedTLI``/etc. in - ``node_active_protocol.c``); -- on the **primary**: refuse and report a benign "not yet" status *unless* - the operation's ``allow_failover`` was set, in which case it requests a - promotion the same way ``pg_autoctl enable maintenance - --allow-failover`` already does today (reusing - ``perform_failover()``), and only applies its own pending change once - demoted. - -This is deliberately the *same* sequence as the manual runbook in -:ref:`wal_level_rollout` -- the design doesn't invent new mechanics, it -automates the existing ones end to end. - -**Client CLI surface** (new subcommands, following the existing -``pg_autoctl perform failover`` / ``enable maintenance`` naming): - -.. code-block:: bash - - pg_autoctl perform rolling-change --formation default \ - --guc wal_level=logical [--allow-failover] - - pg_autoctl show operations [--formation default] - pg_autoctl show operation - pg_autoctl operation unblock [--step ] - pg_autoctl operation cancel - -``perform rolling-change`` is a thin wrapper around -``start_operation()``; ``show operations``/``show operation`` wrap -``get_operation()``/``list_operations()``; the two ``operation`` -subcommands wrap ``unblock_operation()``/``cancel_operation()``. None of -these need the client to stay connected or keep polling itself -- all the -state lives on the monitor, same as every other pg_auto_failover -operation, so the CLI can be re-run from anywhere, at any time, to check -or act on an in-flight operation. - -Open questions ---------------- - -- **Rollback semantics are necessarily per-``kind``, not generic.** - "Cancel" can always stop handing out *new* pending changes, but a step - already applied (Postgres already restarted with the new ``wal_level``) - can only be undone by another forward-moving change (queue - ``wal_level=replica`` as a new pending change on the nodes that already - flipped) -- there is no actual "undo a restart". Each ``kind`` would - need to define both its forward step plan and its compensating one. - -- **Concurrent operations on the same formation/group.** ``start_operation()`` - needs to refuse (or queue) a second operation against a group that - already has one ``running``/``blocked`` -- otherwise two operations - could both decide it's safe to promote at the same time. The existing - ``LockNodeGroup()`` discipline likely extends to this directly, since - it's the same lock ``perform_failover()`` already takes for exactly this - class of race. - -- **Permissions.** ``start_operation()``/``unblock_operation()``/ - ``cancel_operation()`` are operator actions, not something a keeper - should ever call autonomously -- same ``autoctl`` vs. ``autoctl_node`` - role split already used for ``start_maintenance()`` vs. the keeper's own - calls (see :ref:`operations`'s note on the ``autoctl_node`` role being - "not allowed to perform maintenance operations"). - -- **Interaction with manual maintenance mode.** An operator running - ``pg_autoctl enable maintenance`` by hand on a node that's also mid-step - in an orchestrated operation needs a defined precedence -- probably: - manual maintenance always wins, and the operation's own step simply - waits (or is marked ``blocked`` once its timeout elapses), so an - operator always retains the ability to intervene without the two - mechanisms fighting each other. - -- **Scope of "whole-cluster."** The schema above is written per-formation/ - per-group, matching how pg_auto_failover already scopes everything else - (``synchronous_standby_names``, ``synchronized_standby_slots``, - failover). A true multi-formation operation (e.g. a Citus cluster's - coordinator and every worker group) would need an operation to *contain* - other operations, one per group -- not explored further here. diff --git a/tests/tap/schedules/multi-misc.sch b/tests/tap/schedules/multi-misc.sch index 2a2020ae0..a081943fa 100644 --- a/tests/tap/schedules/multi-misc.sch +++ b/tests/tap/schedules/multi-misc.sch @@ -13,8 +13,8 @@ multi_ifdown drop_node_destroy guard_data_loss -# PG17+ only (the spec itself pins image "pg_auto_failover:pg17", -# regardless of which PGVERSION leg runs this schedule): CDC continuity -# (synchronized_standby_slots, failover-enabled logical slots surviving a -# promotion) and the #1188 stale-failover-slot-after-demote_timeout fix. +# PG17+ only (multi-misc only runs on the PG17 matrix leg -- see +# .github/workflows/ci.yml): CDC continuity (synchronized_standby_slots, +# failover-enabled logical slots surviving a promotion) and the #1188 +# stale-failover-slot-after-demote_timeout fix. cdc_continuity diff --git a/tests/tap/specs/cdc_continuity.pgaf b/tests/tap/specs/cdc_continuity.pgaf index 79f890794..b2ea25b46 100644 --- a/tests/tap/specs/cdc_continuity.pgaf +++ b/tests/tap/specs/cdc_continuity.pgaf @@ -23,7 +23,6 @@ cluster { monitor - image "pg_auto_failover:pg17" formation { node1 node2 From 1a3143927dac5913bdbd0af1dbad3a78bb1ad024 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:14:43 +0000 Subject: [PATCH 04/13] Simplify wal_level rollout runbook: stop is enough, disable maintenance restarts disable maintenance's FSM transition for a standby always ends by calling ensure_postgres_service_is_running() (via primary_rewind_to_standby(), reused by fsm_restart_standby()), so Postgres comes back up on its own as part of rejoining -- no separate start/restart step needed, the operator only has to stop it after editing the config. Also notes that pg_autoctl's own Postgres controller explicitly steps aside during enable maintenance (local_postgres_unlink_status_file(), "so that this process won't interfere with whatever the user is doing during maintenance (such as stop Postgres)"), so a plain pg_ctl stop here is already safe and doesn't race pg_autoctl's supervision. --- docs/cdc.rst | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index 3905c846c..c52f08b05 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -151,15 +151,20 @@ For a formation with one primary and one or more standbys: a. ``pg_autoctl enable maintenance`` -- safe: the monitor switches the primary to asynchronous replication and the standby is no longer a - promotion candidate while in this state. + promotion candidate while in this state. pg_autoctl's own Postgres + controller also steps out of the way for the duration, precisely so + that manually stopping/starting Postgres on this node doesn't race + it. b. Apply the change, either directly in ``postgresql.conf`` or via ``ALTER SYSTEM SET wal_level = 'logical';`` (both are safe: pg_auto_failover's own managed settings live in a separate included file, so this never collides with anything it writes). - c. Restart Postgres on that node (``pg_ctl restart``, your init - system, or however you'd restart it for a kernel/Postgres point - release). - d. ``pg_autoctl disable maintenance`` to let it rejoin. + c. Stop Postgres on that node (``pg_ctl stop``, your init system, or + however you'd stop it for a kernel/Postgres point release) -- no + need to start it back up yourself, the next step does that. + d. ``pg_autoctl disable maintenance`` to let it rejoin: this starts + Postgres again as part of bringing the node back, so it comes up + already running with the new ``wal_level``. 2. Once **every standby** has been upgraded, promote one of them and demote the old primary in one controlled step:: From 6b1c1c3f0ae2aee9efc0a5b2254c6d1c956fc0cf Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:15:59 +0000 Subject: [PATCH 05/13] Remove HA-tools comparison from docs/cdc.rst That section was internal research material, not something that belongs in user-facing documentation. --- docs/cdc.rst | 46 ---------------------------------------------- 1 file changed, 46 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index c52f08b05..e6d6e3572 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -185,52 +185,6 @@ back to its original primary/standby layout (or not, if you'd rather leave the promoted standby as the new primary -- nothing above requires failing back). -How this compares to other HA tools ------------------------------------- - -Postgres 17's native slot-sync mechanism is new enough (2024) that most HA -tooling is still catching up. As of writing, only a handful of tools -compute ``synchronized_standby_slots`` automatically instead of leaving it -to the operator to set and maintain by hand: - -================== ========================== ============================== ========================= =============================== -Tool Computes the setting? Derived from Manages slot-sync GUCs? Handles stale slot on demotion? -================== ========================== ============================== ========================= =============================== -pg_auto_failover Yes failover candidates Yes, on by default Yes (this work, issue #1188) -Patroni Yes (merged, unreleased) synchronous standbys No (left to the user) Not addressed -CloudNativePG Yes (opt-in) all HA standbys Partial (user must enable) No -- open issue -EDB PGD No (own Raft mechanism) n/a n/a n/a -repmgr No n/a n/a n/a -================== ========================== ============================== ========================= =============================== - -A few points worth calling out: - - - Patroni's equivalent (`patroni/patroni#3527 - `_) derives the slot list - from the cluster's *synchronous* standbys rather than its failover - *candidates* -- the two sets aren't always the same, and Patroni's - feature is inert in an async-only cluster. It's also opt-in behind - ``synchronous_mode`` and, as of this writing, merged but not in a - released Patroni version yet. - - - CloudNativePG computes the slot list but still requires the operator to - turn on ``sync_replication_slots`` themselves, and has an open issue - (`cloudnative-pg#9969 - `_) for - exactly the stale-slot-after-demotion problem this PR's issue #1188 fix - addresses -- the same underlying Postgres interaction, independently - discovered in a different project. - - - Neither tool -- nor core Postgres -- offers a quorum/"N of M" knob for - ``synchronized_standby_slots``; see the first bullet under - *Limitations* below. - - - EDB Postgres Distributed (PGD/BDR) solves a different, larger problem - (any node in a multi-master cluster can consume the slot, via its own - Raft-based coordination) and is not a direct comparison; it layers its - own ``failover_slot_scope`` on top of, and independently from, the - native PG17 mechanism. - Limitations ----------- From 72f370ef3b71dc7f1c461f4cf14086eee7390882 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:29:56 +0000 Subject: [PATCH 06/13] synchronized_standby_slots: exclude candidates not currently reporting secondary GroupListCandidates() only filters by candidatePriority > 0, regardless of the node's current reportedState -- a candidate the monitor has pushed back to catchingup (lagging, missed a reporting window, ...) was still listed, which would make a failover-enabled logical slot's progress hostage to that node recovering. Filter the candidate list down to AutoFailoverNode.reportedState == secondary. Deliberately checks reportedState, not goalState: an extra standby beyond what number_sync_standbys needs can sit at goalState catchingup indefinitely for quorum bookkeeping alone while genuinely being a fully caught-up secondary, and that node belongs in the list -- this is why the existing regression test's "both standbys are candidates" assertion is unchanged (sss_s2's goalstate is catchingup there, but it had already reported secondary). Adds a new regression scenario exercising the actual lag case: a standby that self-reports catchingup drops out of synchronized_standby_slots, and reappears once it reports secondary again, verified against a live PG17 instance. --- .../expected/synchronized_standby_slots.out | 46 +++++++++++++++-- src/monitor/node_active_protocol.c | 50 +++++++++++++++++-- .../sql/synchronized_standby_slots.sql | 36 +++++++++++-- 3 files changed, 122 insertions(+), 10 deletions(-) diff --git a/src/monitor/expected/synchronized_standby_slots.out b/src/monitor/expected/synchronized_standby_slots.out index 34480ecab..81085b373 100644 --- a/src/monitor/expected/synchronized_standby_slots.out +++ b/src/monitor/expected/synchronized_standby_slots.out @@ -4,9 +4,11 @@ -- Regression test for pgautofailover.synchronized_standby_slots(), the -- Postgres 17+ CDC-continuity companion to synchronous_standby_names: it -- lists the physical replication slot names of the group's current --- failover candidates (AutoFailoverNode.candidatePriority > 0), so that a --- failover-enabled logical replication slot only advances once every node --- that might get promoted has replayed that far. +-- failover candidates (AutoFailoverNode.candidatePriority > 0) that are +-- also currently reporting themselves as secondary (reportedState), so +-- that a failover-enabled logical replication slot only advances once +-- every node that might get promoted right now has replayed that far -- +-- excluding candidates that are lagging or otherwise not caught up. -- -- Same bootstrap sequence as candidate_count_gate.sql: drive the FSM to -- primary + secondary + secondary through pgautofailover.node_active(). @@ -170,6 +172,44 @@ SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); -[ RECORD 1 ]--------------+---------------------------------------------------- synchronized_standby_slots | pgautofailover_standby_39,pgautofailover_standby_40 +-- ── s2 self-reports lagging (catchingup): it drops out despite still being +-- a failover candidate -- this is the lag opt-out: holding the logical +-- slot's progress hostage to a standby that isn't caught up right now +-- would defeat the point of CDC continuity. Note goalstate for s2 was +-- already catchingup before this (an extra standby beyond what +-- number_sync_standbys needs keeps that goal for quorum bookkeeping); +-- what flips here is reportedstate, which is what we actually filter on. +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s2'; +-[ RECORD 1 ]-+----------- +nodename | sss_s2 +goalstate | catchingup +reportedstate | catchingup + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-[ RECORD 1 ]--------------+-------------------------- +synchronized_standby_slots | pgautofailover_standby_39 + +-- ── s2 catches back up and reports secondary again: it's back ───────────── +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); +-[ RECORD 1 ]--------+----------- +assigned_group_state | catchingup + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-[ RECORD 1 ]--------------+---------------------------------------------------- +synchronized_standby_slots | pgautofailover_standby_39,pgautofailover_standby_40 + -- ── s2 opts out of the failover quorum: only s1's slot remains ───────────── SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s2', 0); -[ RECORD 1 ]---------------+-- diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index 6a7095839..e7573d3f7 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -2670,9 +2670,22 @@ synchronous_standby_names(PG_FUNCTION_ARGS) * manually (force-failover), which is already an exceptional, operator- * driven action, so we don't hold back CDC consumers for them. * + * Among those candidates, only the ones that last reported themselves as + * SECONDARY are listed (AutoFailoverNode.reportedState, not goalState: an + * extra standby beyond what number_sync_standbys needs can sit at goal + * catchingup indefinitely for quorum bookkeeping alone, while genuinely + * being a fully caught-up secondary). A candidate the monitor has pushed + * back to catchingup because it actually fell behind or missed its + * reporting window, or that hasn't reached secondary yet at all + * (wait_standby, join_secondary, ...), is excluded: it remains a failover + * candidate in principle, but it isn't caught up right now, and listing it + * would make the logical slot's progress hostage to that node recovering + * -- the opposite of the opt-out this GUC is meant to give us. + * * Returns the empty string when there is nothing to wait on (a single node - * group, or no promotion candidates at all), which disables the feature: - * Postgres treats an empty synchronized_standby_slots as "no requirement". + * group, or no promotion candidates currently reporting secondary), which + * disables the feature: Postgres treats an empty synchronized_standby_slots + * as "no requirement". */ Datum synchronized_standby_slots(PG_FUNCTION_ARGS) @@ -2710,7 +2723,36 @@ synchronized_standby_slots(PG_FUNCTION_ARGS) List *standbyNodesGroupList = AutoFailoverOtherNodesList(primaryNode); List *candidateNodesList = GroupListCandidates(standbyNodesGroupList); - if (list_length(candidateNodesList) == 0) + /* + * Only wait on a candidate that last reported itself as SECONDARY. We + * deliberately check reportedState, not goalState: an extra standby + * beyond what number_sync_standbys needs can sit at goalState + * catchingup indefinitely for quorum bookkeeping reasons alone, while + * genuinely being a fully caught-up secondary (reportedState + * secondary) -- that node belongs in the list. What we want to + * exclude is a candidate the monitor has pushed back to catchingup + * because it actually fell behind or missed its reporting window + * (reportedState catchingup), or one that hasn't reached secondary yet + * at all (wait_standby, join_secondary, ...). Holding a failover- + * enabled logical slot's progress hostage to such a node would defeat + * the point of CDC continuity: the slot would simply stop advancing + * until that node recovers, instead of only waiting on candidates + * actually able to take over right now. + */ + List *readyCandidateNodesList = NIL; + ListCell *candidateCell = NULL; + + foreach(candidateCell, candidateNodesList) + { + AutoFailoverNode *node = (AutoFailoverNode *) lfirst(candidateCell); + + if (node->reportedState == REPLICATION_STATE_SECONDARY) + { + readyCandidateNodesList = lappend(readyCandidateNodesList, node); + } + } + + if (list_length(readyCandidateNodesList) == 0) { PG_RETURN_TEXT_P(cstring_to_text("")); } @@ -2720,7 +2762,7 @@ synchronized_standby_slots(PG_FUNCTION_ARGS) ListCell *nodeCell = NULL; bool firstNode = true; - foreach(nodeCell, candidateNodesList) + foreach(nodeCell, readyCandidateNodesList) { AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); diff --git a/src/monitor/sql/synchronized_standby_slots.sql b/src/monitor/sql/synchronized_standby_slots.sql index 42a70a7c7..efe32c9e5 100644 --- a/src/monitor/sql/synchronized_standby_slots.sql +++ b/src/monitor/sql/synchronized_standby_slots.sql @@ -4,9 +4,11 @@ -- Regression test for pgautofailover.synchronized_standby_slots(), the -- Postgres 17+ CDC-continuity companion to synchronous_standby_names: it -- lists the physical replication slot names of the group's current --- failover candidates (AutoFailoverNode.candidatePriority > 0), so that a --- failover-enabled logical replication slot only advances once every node --- that might get promoted has replayed that far. +-- failover candidates (AutoFailoverNode.candidatePriority > 0) that are +-- also currently reporting themselves as secondary (reportedState), so +-- that a failover-enabled logical replication slot only advances once +-- every node that might get promoted right now has replayed that far -- +-- excluding candidates that are lagging or otherwise not caught up. -- -- Same bootstrap sequence as candidate_count_gate.sql: drive the FSM to -- primary + secondary + secondary through pgautofailover.node_active(). @@ -125,6 +127,34 @@ SELECT nodename, goalstate, reportedstate, candidatepriority SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); +-- ── s2 self-reports lagging (catchingup): it drops out despite still being +-- a failover candidate -- this is the lag opt-out: holding the logical +-- slot's progress hostage to a standby that isn't caught up right now +-- would defeat the point of CDC continuity. Note goalstate for s2 was +-- already catchingup before this (an extra standby beyond what +-- number_sync_standbys needs keeps that goal for quorum bookkeeping); +-- what flips here is reportedstate, which is what we actually filter on. + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'catchingup', + current_lsn => '0/5000'); + +SELECT nodename, goalstate, reportedstate + FROM pgautofailover.node + WHERE formationid = 'sss_test' AND nodename = 'sss_s2'; + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); + +-- ── s2 catches back up and reports secondary again: it's back ───────────── + +SELECT assigned_group_state + FROM pgautofailover.node_active('sss_test', :ns2, 0, + current_group_role => 'secondary', + current_lsn => '0/5000'); + +SELECT pgautofailover.synchronized_standby_slots('sss_test', 0); + -- ── s2 opts out of the failover quorum: only s1's slot remains ───────────── SELECT pgautofailover.set_node_candidate_priority('sss_test', 'sss_s2', 0); From d67cd6d419e164a061e7c2286b1229121ba6c49c Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:55:30 +0000 Subject: [PATCH 07/13] Fix stale FAQ wording, make wal_level's hardcoded default match reality The FAQ still said "you still need to set wal_level = logical yourself" -- stale since wal_level now defaults to logical at node creation (this same PR). Point to the new default/opt-out and rollout sections instead. Also stop lying in the C source: DEFAULT_GUC_SETTINGS_FOR_PG_AUTO_FAILOVER hardcoded wal_level to 'replica' with a comment saying it gets patched to 'logical' by default -- so the array never actually produced 'replica' in the common case, only in the --wal-level replica opt-out. Flip the hardcoded literal to 'logical' (the real default) and only patch the array when a non-Citus node explicitly opted out to 'replica', instead of unconditionally copying and patching on every node creation just to re-assert the same default value. --- docs/faq.rst | 11 +++++-- src/bin/pg_autoctl/primary_standby.c | 44 +++++++++++++--------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index eec23d285..32c515279 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -298,9 +298,14 @@ recent**. See :ref:`cdc_continuity` for the full picture; the short version: a logical slot created with ``failover => true`` keep working after a promotion, instead of being silently left behind on the old primary. - - You still need to set ``wal_level = logical`` yourself (pg_auto_failover - leaves this one to you, since it isn't needed unless you actually use - logical replication) and create your slot(s) with ``failover => true``. + - ``wal_level = logical`` is the default on every node pg_auto_failover + creates (see :ref:`wal_level_default`) -- you only need to set it + yourself when opting back in on a cluster created with + ``--wal-level replica``, or one created before this default existed + (see :ref:`wal_level_rollout`). You still need to create your slot(s) + with ``failover => true`` yourself: pg_auto_failover manages the + infrastructure that keeps them working across a promotion, not slot + creation itself. On Postgres versions before 17, Postgres itself has no mechanism to keep a logical slot's position correct on a standby without losing unconsumed diff --git a/src/bin/pg_autoctl/primary_standby.c b/src/bin/pg_autoctl/primary_standby.c index c615cb388..57491a965 100644 --- a/src/bin/pg_autoctl/primary_standby.c +++ b/src/bin/pg_autoctl/primary_standby.c @@ -47,10 +47,10 @@ static void local_postgres_update_pg_failures_tracking(LocalPostgresServer *post { "port", "5432" }, \ { "max_wal_senders", "12" }, \ { "max_replication_slots", "12" }, \ - { "wal_level", "'replica'" }, /* postgres_add_default_settings() patches \ - * this entry to 'logical' by default, or \ - * --wal-level replica's value, for \ - * non-Citus nodes -- see there. */ \ + { "wal_level", "'logical'" }, /* the default; postgres_add_default_settings() \ + * patches this entry to 'replica' for a \ + * non-Citus node created with \ + * --wal-level replica -- see there. */ \ { "wal_log_hints", "on" }, \ { "wal_sender_timeout", "'30s'" }, \ { "hot_standby_feedback", "on" }, \ @@ -972,32 +972,30 @@ postgres_add_default_settings(LocalPostgresServer *postgres, } /* - * wal_level defaults to 'logical' on every plain (non-Citus) node: - * CDC continuity (see docs/cdc.rst) needs it on any node that might - * ever become primary, which in pg_auto_failover is every node in the - * group. Citus nodes already force 'logical' unconditionally above, - * for Citus's own unrelated needs, so they're left out of this - * override. --wal-level replica at node creation time (pgSetup->walLevel) - * opts back out, for clusters that know they'll never use CDC and - * would rather not pay wal_level=logical's WAL-volume and catalog - * xmin-retention cost. + * wal_level already defaults to 'logical' in the non-Citus + * default_settings arrays above: CDC continuity (see docs/cdc.rst) + * needs it on any node that might ever become primary, which in + * pg_auto_failover is every node in the group. Citus nodes already + * force 'logical' unconditionally for Citus's own unrelated needs, so + * they're left out of this override entirely. * - * The static default_settings arrays above are copied into a local, - * patched array here rather than hardcoding the value into each of - * them, so there's exactly one place that decides the actual value - * written to postgresql.conf. + * --wal-level replica at node creation time (pgSetup->walLevel) is the + * one case that needs patching: a non-Citus node opting back out, for + * clusters that know they'll never use CDC and would rather not pay + * wal_level=logical's WAL-volume and catalog xmin-retention cost. The + * static default_settings array is copied into a local, patched array + * here rather than hardcoding the opt-out into it, so there's exactly + * one place that decides the actual value written to postgresql.conf. */ - if (!IS_CITUS_INSTANCE_KIND(postgres->pgKind)) + if (!IS_CITUS_INSTANCE_KIND(postgres->pgKind) && + !IS_EMPTY_STRING_BUFFER(pgSetup->walLevel) && + strcmp(pgSetup->walLevel, "logical") != 0) { - const char *requestedWalLevel = - IS_EMPTY_STRING_BUFFER(pgSetup->walLevel) - ? "logical" - : pgSetup->walLevel; int i = 0; int maxEntries = (int) (sizeof(patchedSettings) / sizeof(patchedSettings[0])); - sformat(walLevelValue, sizeof(walLevelValue), "'%s'", requestedWalLevel); + sformat(walLevelValue, sizeof(walLevelValue), "'%s'", pgSetup->walLevel); for (i = 0; default_settings[i].name != NULL && i < maxEntries - 1; i++) { From c65be23d02b15999df0cc517a847b4aafdd6ec5a Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Tue, 13 Oct 2026 21:58:05 +0000 Subject: [PATCH 08/13] Simplify wal_level rollout runbook using pg_autoctl restart postgres Replaces the four manual steps per node (enable maintenance, edit config, stop Postgres, disable maintenance) with the new pg_autoctl restart postgres --maintenance [--allow-failover] command from a separate, in-progress PR: edit the config, then one command per node. --allow-failover is still required for the primary, same gate as enable maintenance itself. --- docs/cdc.rst | 54 +++++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index e6d6e3572..5e17efd9a 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -145,40 +145,38 @@ upgrade rather than a GUC change). The one planned switchover below costs the same single, short write-unavailability window as any manual switchover with this tool, nothing more. -For a formation with one primary and one or more standbys: +For a formation with one primary and one or more standbys, using +``pg_autoctl restart postgres`` (a single command that enters maintenance, +restarts Postgres, and leaves maintenance again, since nothing needs to +happen while Postgres is down for a plain configuration change): 1. For each **standby**, one at a time: - a. ``pg_autoctl enable maintenance`` -- safe: the monitor switches the - primary to asynchronous replication and the standby is no longer a - promotion candidate while in this state. pg_autoctl's own Postgres - controller also steps out of the way for the duration, precisely so - that manually stopping/starting Postgres on this node doesn't race - it. - b. Apply the change, either directly in ``postgresql.conf`` or via + a. Apply the change, either directly in ``postgresql.conf`` or via ``ALTER SYSTEM SET wal_level = 'logical';`` (both are safe: pg_auto_failover's own managed settings live in a separate included file, so this never collides with anything it writes). - c. Stop Postgres on that node (``pg_ctl stop``, your init system, or - however you'd stop it for a kernel/Postgres point release) -- no - need to start it back up yourself, the next step does that. - d. ``pg_autoctl disable maintenance`` to let it rejoin: this starts - Postgres again as part of bringing the node back, so it comes up - already running with the new ``wal_level``. - - 2. Once **every standby** has been upgraded, promote one of them and - demote the old primary in one controlled step:: - - pg_autoctl enable maintenance --allow-failover - - This runs the same promotion path as a normal switchover and leaves - the old primary in ``maintenance`` state as a standby -- this is the - one and only planned write-unavailability window in the whole - procedure. - - 3. Apply steps 1b-1d to the now-demoted former primary, then - ``pg_autoctl disable maintenance`` to bring it back as a regular - standby. + b. :: + + pg_autoctl restart postgres --maintenance --pgdata + + This enables maintenance (safe: the monitor switches the primary to + asynchronous replication and the standby is no longer a promotion + candidate while in this state), restarts Postgres so it comes back + up with the new ``wal_level``, then disables maintenance again so + the standby rejoins. + + 2. Once **every standby** has been upgraded, apply the same configuration + change to the primary, then restart it with ``--allow-failover`` added:: + + pg_autoctl restart postgres --maintenance --allow-failover --pgdata + + ``--allow-failover`` is required here: entering maintenance on a + primary means promoting a standby first, the same promotion path as a + normal switchover. This is the one and only planned + write-unavailability window in the whole procedure. Once the restart + completes, the old primary rejoins as a regular standby, already + running with the new ``wal_level``. At the end, every node has ``wal_level = logical`` and the formation is back to its original primary/standby layout (or not, if you'd rather From baa8caa2a1b9600e1bb805c61bfc2adc8d900ba6 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Wed, 14 Oct 2026 12:36:38 +0000 Subject: [PATCH 09/13] Link docs/cdc.rst's wal_level runbook to the merged restart postgres docs pg_autoctl restart postgres (#1198) is now on main after rebasing this branch on top of it. Turn the plain-text command name into a proper :ref: to its manual page, and point to the Operations guide's own restart_postgres section for more worked examples of the same command (including the --exec case, which doesn't apply to a plain wal_level edit but is worth knowing about). --- docs/cdc.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index 5e17efd9a..72b1e8bc5 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -146,9 +146,12 @@ the same single, short write-unavailability window as any manual switchover with this tool, nothing more. For a formation with one primary and one or more standbys, using -``pg_autoctl restart postgres`` (a single command that enters maintenance, -restarts Postgres, and leaves maintenance again, since nothing needs to -happen while Postgres is down for a plain configuration change): +:ref:`pg_autoctl_restart_postgres` (a single command that enters +maintenance, restarts Postgres, and leaves maintenance again, since +nothing needs to happen while Postgres is down for a plain configuration +change -- see :ref:`restart_postgres` for more worked examples of this +command, including one that does need something to run while Postgres is +down): 1. For each **standby**, one at a time: From 30a940699ff51a0a165f394a96f3ce3fd9dff7ee Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Wed, 14 Oct 2026 12:46:08 +0000 Subject: [PATCH 10/13] Dedupe synchronized_standby_slots code paths against their sync_standby_names siblings Same methodology as the restart postgres PR (#1198) review: this PR introduced four near-duplicates of pre-existing functions, each differing only in which monitor/SQL function to call and a setting name used in error messages or JSON output. Extracted the shared logic into one new helper per layer, keeping each pair's own thin wrapper: - node_active_protocol.c: synchronized_standby_slots() duplicated the "build a comma-separated pgautofailover_standby_ list" loop from synchronous_standby_names(). Extracted AppendStandbySlotNames(). - monitor.c: monitor_synchronized_standby_slots() duplicated monitor_synchronous_standby_names()'s entire body (same SQL execution, same error handling) bar the SQL function name and setting name. Extracted monitor_get_group_setting_from_sql(). - cli_show.c: cli_show_synchronized_standby_slots() duplicated cli_show_standby_names()'s entire body (same fetch, same JSON/plain printing) bar the monitor function and setting name. Extracted cli_show_monitor_computed_setting(). - fsm_transition.c: fsm_apply_settings() had two copies of the same "fetch a monitor-computed setting into a buffer, or fall back to a default when there's no monitor" block, one for synchronous_standby_names and one for synchronized_standby_slots. Extracted fsm_fetch_monitor_setting(), taking the monitor function as a function pointer (both have the exact same signature). No behavior change. Verified: the monitor's own synchronized_standby_slots regression test still produces byte-identical output against a live PG17 instance, and a real 2-node cluster still correctly shows synchronous_standby_names, synchronized_standby_slots, and wal_level after the refactor. --- src/bin/pg_autoctl/cli_show.c | 106 ++++++++++++--------------- src/bin/pg_autoctl/fsm_transition.c | 106 ++++++++++++++++----------- src/bin/pg_autoctl/monitor.c | 109 ++++++++++++---------------- src/monitor/node_active_protocol.c | 66 ++++++++--------- 4 files changed, 193 insertions(+), 194 deletions(-) diff --git a/src/bin/pg_autoctl/cli_show.c b/src/bin/pg_autoctl/cli_show.c index 5b298493a..5d0c70496 100644 --- a/src/bin/pg_autoctl/cli_show.c +++ b/src/bin/pg_autoctl/cli_show.c @@ -891,29 +891,39 @@ cli_show_standby_names_getopts(int argc, char **argv) /* - * cli_show_standby_names prints the synchronous_standby_names setting value - * for a given group (in a known formation). + * cli_show_monitor_computed_setting fetches a group-wide, monitor-computed + * setting value (synchronous_standby_names, synchronized_standby_slots, + * ...) via fetchFunction and prints it, in JSON or plain text depending on + * outputJSON. settingName is both the JSON key and what's named in the + * error message, so callers reusing this pass the same string Postgres + * itself uses for the GUC. + * + * Shared by cli_show_standby_names() and cli_show_synchronized_standby_slots() + * below, which otherwise only differ in which monitor function to call and + * which setting name to print under. */ static void -cli_show_standby_names(int argc, char **argv) +cli_show_monitor_computed_setting(KeeperConfig *config, + bool (*fetchFunction) (Monitor *monitor, + char *formation, + int groupId, + char *buffer, + int size), + const char *settingName) { - KeeperConfig config = keeperOptions; Monitor monitor = { 0 }; - char synchronous_standby_names[BUFSIZE] = { 0 }; + char settingValue[BUFSIZE] = { 0 }; - (void) cli_monitor_init_from_option_or_config(&monitor, &config); + (void) cli_monitor_init_from_option_or_config(&monitor, config); - (void) cli_set_groupId(&monitor, &config); + (void) cli_set_groupId(&monitor, config); - if (!monitor_synchronous_standby_names( - &monitor, - config.formation, - config.groupId, - synchronous_standby_names, - BUFSIZE)) + if (!(*fetchFunction)(&monitor, config->formation, config->groupId, + settingValue, BUFSIZE)) { - log_fatal("Failed to get the synchronous_standby_names setting value " - " from the monitor, see above for details"); + log_fatal("Failed to get the %s setting value " + " from the monitor, see above for details", + settingName); exit(EXIT_CODE_MONITOR); } @@ -922,22 +932,35 @@ cli_show_standby_names(int argc, char **argv) JSON_Value *js = json_value_init_object(); JSON_Object *jsObj = json_value_get_object(js); - json_object_set_string(jsObj, "formation", config.formation); - json_object_set_number(jsObj, "group", (double) config.groupId); - json_object_set_string(jsObj, - "synchronous_standby_names", - synchronous_standby_names); + json_object_set_string(jsObj, "formation", config->formation); + json_object_set_number(jsObj, "group", (double) config->groupId); + json_object_set_string(jsObj, settingName, settingValue); (void) cli_pprint_json(js); } else { - /* current synchronous_standby_names might be an empty string */ - (void) fformat(stdout, "'%s'\n", synchronous_standby_names); + /* current setting value might be an empty string */ + (void) fformat(stdout, "'%s'\n", settingValue); } } +/* + * cli_show_standby_names prints the synchronous_standby_names setting value + * for a given group (in a known formation). + */ +static void +cli_show_standby_names(int argc, char **argv) +{ + KeeperConfig config = keeperOptions; + + (void) cli_show_monitor_computed_setting(&config, + monitor_synchronous_standby_names, + "synchronous_standby_names"); +} + + /* * cli_show_synchronized_standby_slots prints the Postgres 17+ * synchronized_standby_slots setting value for a given group (in a known @@ -949,43 +972,10 @@ static void cli_show_synchronized_standby_slots(int argc, char **argv) { KeeperConfig config = keeperOptions; - Monitor monitor = { 0 }; - char synchronized_standby_slots[BUFSIZE] = { 0 }; - (void) cli_monitor_init_from_option_or_config(&monitor, &config); - - (void) cli_set_groupId(&monitor, &config); - - if (!monitor_synchronized_standby_slots( - &monitor, - config.formation, - config.groupId, - synchronized_standby_slots, - BUFSIZE)) - { - log_fatal("Failed to get the synchronized_standby_slots setting value " - " from the monitor, see above for details"); - exit(EXIT_CODE_MONITOR); - } - - if (outputJSON) - { - JSON_Value *js = json_value_init_object(); - JSON_Object *jsObj = json_value_get_object(js); - - json_object_set_string(jsObj, "formation", config.formation); - json_object_set_number(jsObj, "group", (double) config.groupId); - json_object_set_string(jsObj, - "synchronized_standby_slots", - synchronized_standby_slots); - - (void) cli_pprint_json(js); - } - else - { - /* current synchronized_standby_slots might be an empty string */ - (void) fformat(stdout, "'%s'\n", synchronized_standby_slots); - } + (void) cli_show_monitor_computed_setting(&config, + monitor_synchronized_standby_slots, + "synchronized_standby_slots"); } diff --git a/src/bin/pg_autoctl/fsm_transition.c b/src/bin/pg_autoctl/fsm_transition.c index c2778d4cd..37f6f40c3 100644 --- a/src/bin/pg_autoctl/fsm_transition.c +++ b/src/bin/pg_autoctl/fsm_transition.c @@ -632,6 +632,54 @@ fsm_enable_sync_rep(Keeper *keeper) } +/* + * fsm_fetch_monitor_setting fetches a group-wide, monitor-computed setting + * value (synchronous_standby_names, synchronized_standby_slots, ...) into + * the given buffer via monitorFetchFunction, or falls back to + * defaultValueWhenNoMonitor when the monitor is disabled for this node. + * settingName is only used to word the error message. + * + * Shared by fsm_apply_settings() below for both settings it applies, which + * otherwise only differ in which monitor function to call, which buffer to + * fill, and what to fall back to without a monitor. + */ +static bool +fsm_fetch_monitor_setting(Keeper *keeper, + bool (*monitorFetchFunction) (Monitor *monitor, + char *formation, + int groupId, + char *buffer, + int size), + char *buffer, + size_t bufferSize, + const char *defaultValueWhenNoMonitor, + const char *settingName) +{ + Monitor *monitor = &(keeper->monitor); + KeeperConfig *config = &(keeper->config); + + if (config->monitorDisabled) + { + strlcpy(buffer, defaultValueWhenNoMonitor, bufferSize); + return true; + } + + if (!(*monitorFetchFunction)(monitor, + config->formation, + keeper->state.current_group, + buffer, + bufferSize)) + { + log_error("Failed to get the %s value from the monitor, " + "see above for details", + settingName); + return false; + } + + return true; +} + + /* * fsm_apply_settings is used when a pg_auto_failover setting has changed, such * as number_sync_standbys or node priorities and replication quorum @@ -643,31 +691,18 @@ fsm_enable_sync_rep(Keeper *keeper) bool fsm_apply_settings(Keeper *keeper) { - Monitor *monitor = &(keeper->monitor); - KeeperConfig *config = &(keeper->config); LocalPostgresServer *postgres = &(keeper->postgres); - /* get synchronous_standby_names value from the monitor */ - if (!config->monitorDisabled) + /* get synchronous_standby_names value from the monitor, '*' without one */ + if (!fsm_fetch_monitor_setting(keeper, + monitor_synchronous_standby_names, + postgres->synchronousStandbyNames, + sizeof(postgres->synchronousStandbyNames), + "*", + "synchronous_standby_names")) { - if (!monitor_synchronous_standby_names( - monitor, - config->formation, - keeper->state.current_group, - postgres->synchronousStandbyNames, - sizeof(postgres->synchronousStandbyNames))) - { - log_error("Failed to enable synchronous replication because " - "we failed to get the synchronous_standby_names value " - "from the monitor, see above for details"); - return false; - } - } - else - { - /* no monitor: use the generic value '*' */ - strlcpy(postgres->synchronousStandbyNames, "*", - sizeof(postgres->synchronousStandbyNames)); + /* errors have already been logged */ + return false; } if (!primary_set_synchronous_standby_names(postgres)) @@ -684,26 +719,15 @@ fsm_apply_settings(Keeper *keeper) * primary_set_synchronized_standby_slots() is itself a no-op before * Postgres 17, so we always fetch the value and let it decide. */ - if (!config->monitorDisabled) + if (!fsm_fetch_monitor_setting(keeper, + monitor_synchronized_standby_slots, + postgres->synchronizedStandbySlots, + sizeof(postgres->synchronizedStandbySlots), + "", + "synchronized_standby_slots")) { - if (!monitor_synchronized_standby_slots( - monitor, - config->formation, - keeper->state.current_group, - postgres->synchronizedStandbySlots, - sizeof(postgres->synchronizedStandbySlots))) - { - log_error("Failed to set synchronized_standby_slots because " - "we failed to get its value from the monitor, " - "see above for details"); - return false; - } - } - else - { - /* no monitor: nothing to synchronize against */ - strlcpy(postgres->synchronizedStandbySlots, "", - sizeof(postgres->synchronizedStandbySlots)); + /* errors have already been logged */ + return false; } return primary_set_synchronized_standby_slots(postgres); diff --git a/src/bin/pg_autoctl/monitor.c b/src/bin/pg_autoctl/monitor.c index 48cb0596f..9cc1e6b49 100644 --- a/src/bin/pg_autoctl/monitor.c +++ b/src/bin/pg_autoctl/monitor.c @@ -4002,23 +4002,26 @@ monitor_print_formation_settings_as_json(Monitor *monitor, char *formation) /* - * monitor_synchronous_standby_names returns the value for the Postgres - * parameter "synchronous_standby_names" to use for a given group. The setting - * is computed on the monitor depending on the current values of the formation - * number_sync_standbys and each node's candidate priority and replication - * quorum properties. + * monitor_get_group_setting_from_sql runs the given single-row, single + * text-column SQL query (taking formation and groupId as its two + * parameters, in that order) and copies the result into buffer. settingName + * is only used to word the error messages. + * + * Shared by monitor_synchronous_standby_names() and + * monitor_synchronized_standby_slots() below, which otherwise only differ + * in which monitor SQL function to call and what to name the setting in + * their error messages. */ -bool -monitor_synchronous_standby_names(Monitor *monitor, - char *formation, int groupId, - char *synchronous_standby_names, int size) +static bool +monitor_get_group_setting_from_sql(Monitor *monitor, + const char *sql, + const char *settingName, + char *formation, int groupId, + char *buffer, int size) { PGSQL *pgsql = &monitor->pgsql; SingleValueResultContext context = { { 0 }, PGSQL_RESULT_STRING, false }; - const char *sql = - "select pgautofailover.synchronous_standby_names($1, $2)"; - int paramCount = 2; Oid paramTypes[2] = { TEXTOID, INT4OID }; const char *paramValues[2] = { 0 }; @@ -4031,9 +4034,9 @@ monitor_synchronous_standby_names(Monitor *monitor, paramCount, paramTypes, paramValues, &context, &parseSingleValueResult)) { - log_error("Failed to get the synchronous_standby_names setting value " + log_error("Failed to get the %s setting value " " from the monitor for formation %s and group %d", - formation, groupId); + settingName, formation, groupId); if (context.strVal) { free(context.strVal); @@ -4043,10 +4046,10 @@ monitor_synchronous_standby_names(Monitor *monitor, if (!context.parsedOk) { - log_error("Failed to get the synchronous_standby_names setting value " + log_error("Failed to get the %s setting value " " from the monitor for formation %s and group %d," "see above for details", - formation, groupId); + settingName, formation, groupId); if (context.strVal) { free(context.strVal); @@ -4054,13 +4057,34 @@ monitor_synchronous_standby_names(Monitor *monitor, return false; } - strlcpy(synchronous_standby_names, context.strVal, size); + strlcpy(buffer, context.strVal, size); free(context.strVal); return true; } +/* + * monitor_synchronous_standby_names returns the value for the Postgres + * parameter "synchronous_standby_names" to use for a given group. The setting + * is computed on the monitor depending on the current values of the formation + * number_sync_standbys and each node's candidate priority and replication + * quorum properties. + */ +bool +monitor_synchronous_standby_names(Monitor *monitor, + char *formation, int groupId, + char *synchronous_standby_names, int size) +{ + return monitor_get_group_setting_from_sql( + monitor, + "select pgautofailover.synchronous_standby_names($1, $2)", + "synchronous_standby_names", + formation, groupId, + synchronous_standby_names, size); +} + + /* * monitor_synchronized_standby_slots returns the value for the Postgres 17+ * parameter "synchronized_standby_slots" to use for a given group, computed @@ -4076,51 +4100,12 @@ monitor_synchronized_standby_slots(Monitor *monitor, char *formation, int groupId, char *synchronized_standby_slots, int size) { - PGSQL *pgsql = &monitor->pgsql; - SingleValueResultContext context = { { 0 }, PGSQL_RESULT_STRING, false }; - - const char *sql = - "select pgautofailover.synchronized_standby_slots($1, $2)"; - - int paramCount = 2; - Oid paramTypes[2] = { TEXTOID, INT4OID }; - const char *paramValues[2] = { 0 }; - IntString myGroupIdString = intToString(groupId); - - paramValues[0] = formation; - paramValues[1] = myGroupIdString.strValue; - - if (!pgsql_execute_with_params(pgsql, sql, - paramCount, paramTypes, paramValues, - &context, &parseSingleValueResult)) - { - log_error("Failed to get the synchronized_standby_slots setting value " - " from the monitor for formation %s and group %d", - formation, groupId); - if (context.strVal) - { - free(context.strVal); - } - return false; - } - - if (!context.parsedOk) - { - log_error("Failed to get the synchronized_standby_slots setting value " - " from the monitor for formation %s and group %d," - "see above for details", - formation, groupId); - if (context.strVal) - { - free(context.strVal); - } - return false; - } - - strlcpy(synchronized_standby_slots, context.strVal, size); - free(context.strVal); - - return true; + return monitor_get_group_setting_from_sql( + monitor, + "select pgautofailover.synchronized_standby_slots($1, $2)", + "synchronized_standby_slots", + formation, groupId, + synchronized_standby_slots, size); } diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index e7573d3f7..2704e49e4 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -2511,6 +2511,37 @@ update_node_metadata(PG_FUNCTION_ARGS) } +/* + * AppendStandbySlotNames appends the pgautofailover_standby_ names + * of every node in nodeList to buf, in list order, joined by separator. + * Shared by synchronous_standby_names() and synchronized_standby_slots() + * below, which otherwise only differ in which node list they build this + * from and how they wrap the result (synchronous_standby_names() wraps it + * in "ANY N (...)" itself, around this call). + */ +static void +AppendStandbySlotNames(StringInfo buf, List *nodeList, const char *separator) +{ + ListCell *nodeCell = NULL; + bool firstNode = true; + + foreach(nodeCell, nodeList) + { + AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); + + appendStringInfo(buf, + "%spgautofailover_standby_%lld", + firstNode ? "" : separator, + (long long) node->nodeId); + + if (firstNode) + { + firstNode = false; + } + } +} + + /* * synchronous_standby_names returns the synchronous_standby_names parameter * value for a given Postgres service group in a given formation. @@ -2621,25 +2652,9 @@ synchronous_standby_names(PG_FUNCTION_ARGS) : formation->number_sync_standbys; StringInfo sbnames = makeStringInfo(); - ListCell *nodeCell = NULL; - bool firstNode = true; appendStringInfo(sbnames, "ANY %d (", number_sync_standbys); - - foreach(nodeCell, syncStandbyNodesGroupList) - { - AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); - - appendStringInfo(sbnames, - "%spgautofailover_standby_%lld", - firstNode ? "" : ", ", - (long long) node->nodeId); - - if (firstNode) - { - firstNode = false; - } - } + AppendStandbySlotNames(sbnames, syncStandbyNodesGroupList, ", "); appendStringInfoString(sbnames, ")"); PG_RETURN_TEXT_P(cstring_to_text(sbnames->data)); @@ -2759,23 +2774,8 @@ synchronized_standby_slots(PG_FUNCTION_ARGS) else { StringInfo slotNames = makeStringInfo(); - ListCell *nodeCell = NULL; - bool firstNode = true; - foreach(nodeCell, readyCandidateNodesList) - { - AutoFailoverNode *node = (AutoFailoverNode *) lfirst(nodeCell); - - appendStringInfo(slotNames, - "%spgautofailover_standby_%lld", - firstNode ? "" : ",", - (long long) node->nodeId); - - if (firstNode) - { - firstNode = false; - } - } + AppendStandbySlotNames(slotNames, readyCandidateNodesList, ","); PG_RETURN_TEXT_P(cstring_to_text(slotNames->data)); } From 85e47660ea229f39569a15334152235ada34f375 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Wed, 14 Oct 2026 13:10:58 +0000 Subject: [PATCH 11/13] Fix citus_indent style violations (space before paren) --- src/bin/pg_autoctl/cli_show.c | 10 +++++----- src/bin/pg_autoctl/fsm_transition.c | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bin/pg_autoctl/cli_show.c b/src/bin/pg_autoctl/cli_show.c index 5d0c70496..9d8645b1e 100644 --- a/src/bin/pg_autoctl/cli_show.c +++ b/src/bin/pg_autoctl/cli_show.c @@ -904,11 +904,11 @@ cli_show_standby_names_getopts(int argc, char **argv) */ static void cli_show_monitor_computed_setting(KeeperConfig *config, - bool (*fetchFunction) (Monitor *monitor, - char *formation, - int groupId, - char *buffer, - int size), + bool (*fetchFunction)(Monitor *monitor, + char *formation, + int groupId, + char *buffer, + int size), const char *settingName) { Monitor monitor = { 0 }; diff --git a/src/bin/pg_autoctl/fsm_transition.c b/src/bin/pg_autoctl/fsm_transition.c index 37f6f40c3..96526aec9 100644 --- a/src/bin/pg_autoctl/fsm_transition.c +++ b/src/bin/pg_autoctl/fsm_transition.c @@ -645,11 +645,11 @@ fsm_enable_sync_rep(Keeper *keeper) */ static bool fsm_fetch_monitor_setting(Keeper *keeper, - bool (*monitorFetchFunction) (Monitor *monitor, - char *formation, - int groupId, - char *buffer, - int size), + bool (*monitorFetchFunction)(Monitor *monitor, + char *formation, + int groupId, + char *buffer, + int size), char *buffer, size_t bufferSize, const char *defaultValueWhenNoMonitor, From 644556a0a024e3051a4f757968deac5049aa3c25 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Wed, 14 Oct 2026 14:38:22 +0000 Subject: [PATCH 12/13] Update CDC docs limitation: lagging candidates now self-exclude The "No N of M" limitation bullet was stale: it described a permanently lagging/unreachable candidate as permanently stalling decoding, before the fix that filters synchronized_standby_slots down to candidates currently reporting themselves as secondary. A lagging candidate now drops out of the list on its own (pushed to catchingup by the monitor) and reappears once it catches back up, so the remaining limitation is narrower: there's still no knob to require only some of several simultaneously-healthy candidates, not that a single bad node blocks everything indefinitely. --- docs/cdc.rst | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index 72b1e8bc5..5f84126b9 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -208,19 +208,22 @@ Limitations Citus needs more than this feature to give CDC consumers a coherent single stream, and is intentionally out of scope here. - - **No "N of M" / per-candidate opt-out.** ``synchronized_standby_slots`` - lists *every* current failover candidate unconditionally: Postgres - itself blocks a failover-enabled logical slot from advancing until - *all* of them confirm, with no quorum knob. A permanently lagging or - unreachable candidate therefore permanently stalls logical decoding - (and piles up WAL on the primary) until it catches up, is dropped as a - candidate, or is promoted. This is a known, documented trade-off of the - underlying Postgres feature, not something pg_auto_failover adds; other - tools that compute this setting (Patroni, CloudNativePG) have the same - constraint, since core provides no way to ask for "N of M". If this - bites you in practice, lowering the affected node's candidate priority - to ``0`` removes it from the list (at the cost of no longer being an - automatic failover candidate). + - **No "N of M" quorum knob.** Among the candidates it lists, + ``synchronized_standby_slots`` requires *all* of them to confirm before + a failover-enabled logical slot advances -- Postgres provides no + quorum knob here, unlike ``synchronous_standby_names``'s ``ANY N (...)`` + syntax. This does not mean a lagging or unreachable candidate + permanently stalls decoding, though: pg_auto_failover only lists a + candidate while it's currently a converged secondary, so one that + falls behind or stops reporting in (pushed back to ``catchingup`` by + the monitor) drops out of the list on its own, and the slot keeps + advancing against whichever candidates are still caught up -- it + reappears automatically once that node catches back up and reports + secondary again. What's still missing is an explicit operator-facing + knob to require only some of several simultaneously-healthy candidates + to confirm; to exclude a node from the list permanently, regardless of + its replication state, lower its candidate priority to ``0`` (at the + cost of no longer being an automatic failover candidate). - **A node flip-flopping back to primary can lose its own originally-created slot.** If the *same* node that originally created a From 82023cad48c7e7ef56fd25ab889ae6f28a080012 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Wed, 14 Oct 2026 14:50:50 +0000 Subject: [PATCH 13/13] Fix wal_level runbook: prose must precede the command, not follow it Step 1.b was just "b. ::" with a bare command block, and the explanation of what the command does trailed after it instead of introducing it -- a reader hit the command with zero context first. Restructured to match step 2's existing pattern: explain what's about to happen, then show the command. --- docs/cdc.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/cdc.rst b/docs/cdc.rst index 5f84126b9..cad1b06d9 100644 --- a/docs/cdc.rst +++ b/docs/cdc.rst @@ -159,16 +159,15 @@ down): ``ALTER SYSTEM SET wal_level = 'logical';`` (both are safe: pg_auto_failover's own managed settings live in a separate included file, so this never collides with anything it writes). - b. :: + b. Restart Postgres through maintenance mode: this enables maintenance + (safe: the monitor switches the primary to asynchronous replication + and the standby is no longer a promotion candidate while in this + state), restarts Postgres so it comes back up with the new + ``wal_level``, then disables maintenance again so the standby + rejoins:: pg_autoctl restart postgres --maintenance --pgdata - This enables maintenance (safe: the monitor switches the primary to - asynchronous replication and the standby is no longer a promotion - candidate while in this state), restarts Postgres so it comes back - up with the new ``wal_level``, then disables maintenance again so - the standby rejoins. - 2. Once **every standby** has been upgraded, apply the same configuration change to the primary, then restart it with ``--allow-failover`` added::