Archiving & Disaster Recovery: schema through base-backup generation (M1-M5) - #1186
Draft
dimitri wants to merge 78 commits into
Draft
Archiving & Disaster Recovery: schema through base-backup generation (M1-M5)#1186dimitri wants to merge 78 commits into
dimitri wants to merge 78 commits into
Conversation
…or API) Adds the SQL-only foundation for the Archiver process identity / ARCHIVING node membership / base-backup policy / PITR schema described in the archiving-disaster-recovery design doc, milestone 1 (schema + monitor API only -- no service_archiver process involved yet, everything is exercised via direct SQL calls against a plain cluster). - pgautofailover.replication_state gains a new 'archiving' terminal state. - pgautofailover.node gains haspgdata bool, distinguishing ordinary Postgres instances from lightweight ARCHIVING membership rows (a pg_receivewal client, no PGDATA). The old unconditional UNIQUE (nodehost, nodeport) constraint is replaced with a partial unique index scoped to haspgdata rows, since one archiver's (hostname, 0) pair is deliberately shared across every group it serves. - New types, tables and ~26 plpgsql/SQL functions covering: archiver registration and storage targets (local + rclone), formation/group archiver policy (quorum, base-backup policy, replication-quorum eligibility), WAL capture confirmation (wal_archived()/ report_wal_received()), base-backup lifecycle and pruning, warm-standby archiver_node rows with a maxresidentreplay cap, and PITR node lifecycle + command queue. - pgautofailover--2.2--2.3.sql mirrors the same DDL incrementally, since 2.3 hasn't shipped yet; verified end-to-end against a real 1.0 -> ... -> 2.2 -> 2.3 upgrade (including the pre-existing node_nodehost_nodeport_key1 constraint name quirk from two earlier migrations each recreating the table). - New archiving_schema regress test exercising the full schema end-to-end via direct SQL, added at the end of regress_schedule (after cluster_init_failover_rule_attribution, before the dummy_update/ drop_extension/upgrade trio that must stay last) since its expected output pins literal id values tied to its exact position in the shared contrib_regression database, same as every other test in this schedule. Full local regress (20/20) + isolation (6/6) schedules pass, plus a verified real extension upgrade from 2.2 to 2.3.
Adds monitor-side (SQL FSM + C) support for the ARCHIVING replication state, so an ARCHIVING node row (haspgdata = false, created by M1's archiver_add_formation()) is driven through the same node_active() protocol as an ordinary node instead of being stuck at wait_standby forever. No keeper-side/service_archiver work yet -- this is groundwork, verified via node_active() calls made directly against the monitor. - ReplicationState gains REPLICATION_STATE_ARCHIVING (C) / 'archiving' already existed on the SQL enum from M1. - AutoFailoverNode gains hasPgData, populated via TupleToAutoFailoverNode. Looked up by name (SPI_fnumber), not the file's usual hardcoded Anum_ constant: this function is also called against a "RETURNING node.*" tuple descriptor whose physical column order diverges from the explicit SELECT list's logical order once pg_versionnum/pg_version/ pg_versionstring/citus_version are in the mix, so a hardcoded ordinal would silently read the wrong (and wrongly-typed) column for that caller. - MonitorFSM[]: pos 307/309/315/317/319 (report_lsn/wait_standby, primary converged -> secondary/catchingup) gain an explicit hasPgData = TRUE restriction, paired with 5 new hasPgData = FALSE mirror rows (pos 394-398) assigning ARCHIVING instead -- appended after the existing MS-failover cluster since the ordinary rows are numbered with no room between them for 5 more, and the hasPgData split makes their relative order irrelevant to first-match-wins. Pos 367's MS-failover fan-out row (and BuildCandidateList's own C-side secondaryStates list) now also admits ARCHIVING, pulling it into report_lsn during elections exactly like SECONDARY/CATCHINGUP. - system_identifier_is_null_at_init_only loosened to also allow a NULL sysidentifier while reportedstate is 'archiving' or 'report_lsn': an ARCHIVING row never gets a real one. The 2.2--2.3 migration mirror casts the column to text instead of the literals to the enum, since this script's own earlier ADD VALUE 'archiving' and this constraint run in the same ALTER EXTENSION UPDATE transaction and Postgres refuses to create new instances of a not-yet-committed enum value. - keeper_fsm_edges.sql's own "expect zero rows" comment updated: 8 rows are now expected there, a real and currently correct gap -- the monitor side landed first, with no service_archiver/KeeperFSM[] support yet to report ARCHIVING or drive pg_receivewal (next milestone). Verified against a hand-run node_active() scenario (register primary + secondary, converge to primary/secondary, attach an archiver, confirm wait_standby -> archiving instead of catchingup, steady-state archiving stays archiving, replication_quorum = true fans out apply_settings to the primary exactly like an ordinary quorum standby, and rule_pos attribution points at the new rows) in addition to the full regress (20/20) + isolation (6/6) suites and a real 2.2 -> 2.3 extension upgrade.
Adds the keeper-side counterpart to the monitor-side ARCHIVING FSM support (previous commit): KeeperFSM[] rows for WAIT_STANDBY->ARCHIVING/ARCHIVING->REPORT_LSN/REPORT_LSN->ARCHIVING, each dispatching to a new, archiver-specific transition function (fsm_init_archiver/fsm_archiver_report_lsn/fsm_archiver_follow_new_primary, fsm_transition.c) -- mirroring the existing NODE_KIND_CITUS_* pattern of adding separate functions per node kind rather than branching inside the shared ones (fsm_init_standby, keeper_update_pg_state, keeper_ensure_current_state, keeper_node_active_loop stay untouched). New service_archiver.c launches and tracks the one pg_receivewal child an ARCHIVING node keeps running against its group's current primary -- milestone 2's own "colocated fast path" scope (see the Build order in ~/dev/temp/archiving-disaster-recovery.md): pg_receivewal is a real, unmodified Postgres client talking straight to the real primary's own walsender, so no new wire protocol is needed here at all. Not yet wired into supervisor.c's Service/RestartPolicy machinery or a replication slot -- both noted as follow-ups in that file's own header comment. Also: ARCHIVING_STATE added to NodeState (state.h/state.c) and to nodestate_utils.c's nodestateConnectionType() switch (grouped with the other "Postgres known to be stopped" states, since an ARCHIVING row never has a postmaster of its own -- this switch has no default case by design, so a missing case here would have failed the build). Verified: full regress (20/20) + isolation (6/6) suites still pass, and the monitor/keeper reachability cross-check in keeper_fsm_edges.sql -- which the previous commit deliberately left showing 8 unresolved rows, documented as this milestone's own known gap -- now shows zero rows again, both directions, confirming the two sides agree. Live-checked via `pg_autoctl inspect fsm list --json`, which is also how keeper_fsm_edges.json was regenerated (pretty-printed to match the existing file's own review-friendly formatting, not the CLI's compact default). citus_indent and ci/banned.h.sh both pass (the latter caught a raw strerror()/fprintf(stderr) call in service_archiver.c's own exec- failure path, fixed to the project's own log_fatal(..., "%m") convention already used at the other execv() call sites in this codebase).
… (M2 continued)
Adds NODE_KIND_ARCHIVER as a real PgInstanceKind (pgsetup.h/pgsetup.c,
name<->enum both directions), two monitor RPC wrapper functions
(monitor_register_archiver/monitor_archiver_add_formation, monitor.c,
calling M1's own register_archiver()/archiver_add_formation() plpgsql
functions -- not the ordinary C register_node() RPC, since an Archiver is
a process identity, not a (formation, group) membership by itself), and
`pg_autoctl create archiver` (cli_create_node.c): a deliberately minimal,
hand-rolled getopts (not the shared cli_create_node_getopts every ordinary
node kind uses, since that parser's defaults assume a real PostgresSetup
an archiver never has) that registers with the monitor, writes a
KeeperConfig + initial state file (WAIT_STANDBY_STATE, mirroring
archiver_add_formation()'s own starting point), and with --run hands off
to service_archiver_loop() (previous commit).
Verified live against the real monitor RPC layer (not just static
review): registration, formation attachment, and config/state file
writing all confirmed end-to-end against a real running monitor extension
instance, including two real bugs the empirical run caught that manual
review missed --
- config_find_pg_ctl() unconditionally clears pgSetup.pg_ctl before
searching, silently discarding a caller-supplied --pgctl value; fixed
by only calling it when pg_ctl is still empty (also added the missing
--pgctl flag itself -- this dev machine has two pg_ctl on PATH and
needs it to disambiguate, a real scenario, not a test-only one).
- keeper_config_write_file() requires pg_autoctl.role set (validated
against KEEPER_ROLE, not defaulted on write) -- config.role was never
populated, since this path deliberately skips keeper_config_init()'s
ordinary defaults (Postgres-specific probing that doesn't apply here).
`--run`'s actual pg_receivewal launch is still unverified against a real
streaming primary -- needs a real replication-configured Postgres pair,
which is exactly what the next step (pgaftest wiring) provides.
citus_indent and ci/banned.h.sh both pass.
autoctl_node was only ever granted EXECUTE on the function, never SELECT on pgautofailover.basebackup itself, matching every other autoctl_node- callable helper that reads a table it has no direct grant on (e.g. archiver_add_formation) -- get_latest_basebackup was the odd one out. Found via a real end-to-end test of `pg_autoctl archiver serve` against a live monitor: calling it as autoctl_node failed with "permission denied for table basebackup".
The archiver's serving half: a standalone binary (no pg_autoctl/*.c
dependency, only src/bin/common/ and src/bin/lib/log/) that speaks enough
of the real Postgres replication protocol to serve IDENTIFY_SYSTEM, SHOW,
BASE_BACKUP, and a non-standard FETCH_FILE side-channel, backed by an
archiver's local WAL cache and base backups instead of a live postmaster.
No frontend-linkable server-side protocol library exists anywhere in
Postgres (confirmed against pqcomm.c/backend_startup.c/repl_gram.y/
walsender.c, all backend-only) -- this is a genuine reimplementation
guided by that source, not a linking exercise. Two pieces are vendored
near-verbatim since they're already frontend-safe: vendor/tar.c + pgtar.h
(PostgreSQL's own ustar header/checksum logic, src/port/tar.c).
Also ships fetch_client.c / `pg_walsender fetch-file`, the client side of
the FETCH_FILE side-channel, for use as pg_autoctl's own restore_command.
Verified against real, unmodified PostgreSQL client tools:
- psql (replication=1): IDENTIFY_SYSTEM, SHOW wal_segment_size
- pg_basebackup --format=plain -X none --no-manifest: fetched a real
base backup byte-identical to the source, then booted a live Postgres
instance from the result
- pg_walsender fetch-file: fetched a full 16MB WAL segment byte-
identical, plus clean error handling (missing file, path traversal,
unknown route)
See ~/dev/temp/archiving-disaster-recovery.md for the design this
implements milestone 2 of.
`pg_autoctl archiver serve` (cli_archiver.c) is the supervisor verb that
execs pg_walsender as a persistent child (service_archiver_serve.c),
mirroring exactly how service_archiver.c already execs real pg_receivewal
for the outbound WAL-capture direction -- same pattern, new direction.
Keeps pg_walsender's routes file ("<formation>/<group>" -> { walcache,
basebackup }) current, refreshed periodically and on SIGHUP.
The routes file is built from *local* config (formation/groupId/pgSetup.
pgdata), not a monitor round-trip: archiver_add_formation()'s own SQL
inserts the new archiver_node row's pgdata as an empty string, since the
monitor has no way to know an archiver's local WAL cache path -- that's
inherently archiver-host-local information. The one genuinely monitor-
tracked piece is the latest base backup's storage location
(monitor_get_latest_basebackup_location, new in monitor.c).
KeeperConfig gains archiverId/archiverIdStr (keeper_config.h/.c) so a
later, separate `archiver serve` invocation can identify itself to the
monitor -- ini_file.c's INI_INT_T is a plain int, too narrow for a
bigserial id, so this follows citusRoleStr/citusRole's existing string-
plus-parsed-value pattern in the same struct.
Verified against a real, freshly-created cluster (create monitor ->
create postgres -> create archiver -> archiver serve): archiverId
persists and round-trips correctly, the routes file is generated
correctly from live monitor state, pg_walsender starts and serves real
clients through it, and SIGTERM shuts the whole thing down cleanly.
…(M2c)
Completes milestone 2's command surface:
- TIMELINE_HISTORY <tli>: serves a "<tli>.history" file straight out of
the WAL cache directory (RowDescription/DataRow, no COPY involved --
traced from walsender.c's own SendTimeLineHistory()).
- CREATE_REPLICATION_SLOT / READ_REPLICATION_SLOT (physical only): a
slot is a bookkeeping marker file under the WAL cache directory, not
a real Postgres slot -- there's no live server to hold one. Not yet
wired into WAL-retention enforcement (prune_archiver_wal()'s job).
- START_REPLICATION [SLOT <name>] <lsn> TIMELINE <tli>: streams raw WAL
bytes straight from the WAL cache directory. Deliberately does NOT
vendor xlogreader.c: real walsender's own WalSndSegmentOpen just opens
a path computed from TLI+segno and streams bytes -- no WAL *record*
decoding is needed to serve a byte range, only the offset/segment
bookkeeping this file does directly. Handles the actively-growing
(".partial") segment case by polling, matching pg_receivewal's own
producer on the other end of this same protocol.
- wal_dir_scan.c: shared helper -- finds the newest fully-captured WAL
segment and derives its boundary LSN from the filename (XLogFileName
format, fixed 16MB segments). Used by START_REPLICATION's default
position, CREATE_REPLICATION_SLOT's consistent_point, and improves
IDENTIFY_SYSTEM's xlogpos (previously a "0/0" placeholder).
One correctness fix alongside: IDENTIFY_SYSTEM's dbname column must be
NULL for a plain replication=1/true connection (pg_receivewal's style) --
only replication=database (pg_basebackup's style) gets a real dbname back.
Always returning a value broke real pg_receivewal outright ("replication
connection using slot ... is unexpectedly database specific"), caught by
this milestone's own end-to-end testing, not by any narrower unit check.
Verified against real, unmodified PostgreSQL client tools:
- psql: TIMELINE_HISTORY round-trips real file content
- psql: CREATE_REPLICATION_SLOT / READ_REPLICATION_SLOT round-trip
consistent_point/restart_lsn correctly, including the "slot doesn't
exist" all-NULL-row case
- pg_receivewal -S <slot> --endpos=...: streamed a full 16MB WAL segment
byte-identical to the source via START_REPLICATION
(and `pg_autoctl stop`) now supervises an archiver's two
halves together -- WAL capture (service_archiver.c's service_archiver_loop,
outbound pg_receivewal against the primary) and serving (service_archiver_
serve.c's service_archiver_serve_loop, inbound pg_walsender) -- as two real
supervisor.c Service[] entries under one restart-on-crash process tree, the
same way start_keeper() already supervises postgres + node-active together
for an ordinary node (service_archiver_run.c). Dispatched from cli_service.
c's cli_keeper_run(), which already reaches an archiver's config file via
the existing role=keeper path; branches on nodeKind before the Postgres-
instance-specific local_postgres_init()/start_keeper() calls, which don't
apply to an archiver.
Two real bugs surfaced by actually running the full archiver process tree
end-to-end for the first time this session (service_archiver_loop's own
monitor-reporting loop was never previously exercised against a live
monitor for more than a few ticks):
- keeper->postgres.currentLSN was never initialized for an archiver (it
has no real Postgres instance to query it from, so keeper_update_pg_
state() -- the only place that ever set it -- is never called on this
path). node_active()'s own pg_lsn parameter rejected the resulting
empty string outright. Fixed by seeding it to "0/0" once, matching
keeper_update_pg_state()'s own placeholder before a real reading
exists; an archiver's actual capture progress is tracked separately
via archiver_wal, not through this per-node report.
- An ordinary node's own get_other_nodes()/current_state listings now
legitimately include ARCHIVING rows with nodeport = 0 (a deliberate
sentinel, see archiver_add_formation()'s own SQL comment: no
postmaster to be reachable on) -- but monitor.c's node-parsing helpers
treated a parsed port of exactly 0 as an unconditional error, so any
ordinary primary/secondary in a formation with an archiver attached
would fail its own node-active loop entirely. Relaxed the two multi-
node-listing parsers to only reject a genuine parse failure, not the
value 0 itself; left the single-node lookup (which can never
legitimately return an archiver, candidate_priority = 0 excludes it)
unchanged.
Verified against a real, freshly-created cluster (monitor + primary +
archiver): `pg_autoctl run --pgdata archiver1` starts both services
cleanly, the FSM transitions wait_standby -> archiving and real
pg_receivewal starts against the primary, pg_walsender serves real
clients through it, and `pg_autoctl stop` cascades a graceful shutdown
through both services and their own child processes (pg_walsender,
pg_receivewal) with no orphans left behind. Also re-verified the primary
node's own node-active loop, previously broken by the port=0 regression,
now runs cleanly with an archiver attached to its formation.
service_archiver.c gains service_archiver_report_captured_wal(), called once per node_active tick from service_archiver_loop(): it scans the archiver's local WAL cache directory for segments pg_receivewal has completed (i.e. no longer ".partial") since the last one reported, and calls the new monitor_report_wal_received() (monitor.c/.h) -- a thin wrapper around the already-existing pgautofailover.report_wal_received() SQL function -- for each one. This is what actually populates archiver_wal and makes wal_archived() return true; until now nothing in the codebase ever called that SQL function. Also fixes a liveness gap this uncovered: pg_receivewal was only ever (re)started from the FSM transition functions that move a node *into* ARCHIVING_STATE (fsm_init_archiver, fsm_archiver_follow_new_primary). An archiver process restarted while already ARCHIVING (or one whose pg_receivewal child died on its own) had nothing to bring it back up, despite this file's own header comment already describing that as the design. service_archiver_loop() now checks service_archiver_pgreceivewal_is_running() every tick and restarts it when needed, exactly matching that comment. Verified end-to-end against a real monitor + primary + archiver: forced WAL switches on the primary, confirmed archiver_wal gets populated with the correct end-of-segment LSNs and wal_archived() correctly reflects archiver_quorum, confirmed the liveness restart itself by killing and restarting the archiver process while already ARCHIVING. Full SQL regression schedule (src/monitor, 20/20) still passes. Dockerfile: copy pg_walsender into the "run" stage image alongside pg_autoctl -- needed for any archiver node in a pgaftest Docker environment, and a prerequisite for M4's own pgaftest specs.
Adds the "archiver" node kind to pgaftest's own DSL, needed to write any
.pgaf spec that includes an ARCHIVING node:
- test_spec_scan.l/.y: new "archiver" keyword (T_ARCHIVER), usable the
same way "coordinator"/"worker" already are: `archiver1 archiver`
inside a formation{} block.
- compose_gen.c: writes kind = archiver into the node's .ini, and links
service_archiver.c into pgaftest's own SHARED_SRCS (Makefile) so the
binary can drive an archiver node the same way it already drives
postgres/coordinator/worker ones.
- nodespec.c (pg_autoctl, not pgaftest): teaches `pg_autoctl node run
<node.ini>` -- the one command every pgaftest container actually
execs -- to recognize kind = archiver and build the right `pg_autoctl
create archiver` argv. An archiver's own getopts is deliberately
minimal (no --pgport/--ssl-*/--auth/...), so it gets its own argv
branch rather than falling through into the generic postgres-flags
path every other kind shares.
Also fixes a real bug in cli_indent.c's print_node() found while
writing the first archiver spec and round-tripping it through `pgaftest
indent`: the node-kind-to-keyword switch only had cases for coordinator
and worker, so indenting a spec containing an archiver node silently
dropped the "archiver" keyword on write-back, turning it into a plain
postgres node. Added the missing NODE_KIND_ARCHIVER case.
test_spec_parse.c/.h and test_spec_scan.c are bison/flex output,
regenerated from the .y/.l changes above.
First pgaftest spec exercising an ARCHIVING node, covering the two things Milestone 4 adds: - test_001/test_002: forcing WAL segment switches on the primary gets each completed segment reported to the monitor (service_archiver_report_captured_wal(), service_archiver.c) and reflected by pgautofailover.wal_archived() -- the archive_command confirmation check nothing populated before this milestone. - test_002 also exercises the liveness fix in service_archiver_loop(): killing and restarting the archiver process while it's already ARCHIVING must bring pg_receivewal back up on its own, not just on the FSM transition that first enters that state. - test_003: fails node1 over to node2 and confirms the archiver passes through REPORT_LSN_STATE and back to ARCHIVING_STATE (following the new primary), and that segments recorded before the failover are still there afterwards. Segment filenames are asserted directly (a fresh cluster deterministically starts WAL at 000000010000000000000001, and each pg_switch_wal() on an idle test database advances exactly one segment) since autoctl_node has no direct SELECT on archiver_wal -- wal_archived() is the only accessor it can call. Registered in tests/tap/schedule and tests/tap/schedules/node.sch, alongside the other node-lifecycle/FSM specs.
First half of Milestone 5 per the design doc's own Build order ("live
first, then replay/volatile"). New file service_archiver_basebackup.c
adds service_archiver_maybe_generate_basebackup(), called once per tick
from service_archiver_loop() alongside the M4 WAL-report/liveness calls.
Trigger scope for this pass: bootstrap only -- a group with zero
existing base backups (monitor_get_latest_basebackup_location() reports
not-found) gets one immediately. Scheduled/timeline-change/retention
triggers need basebackup_policy wired through the CLI first, a later
milestone.
Target selection follows the design doc's `live` precedence, minus its
warm-standby tier (nothing to select from yet, also later): the first
healthy secondary in the group (monitor_get_nodes(), skipping port == 0
ARCHIVING rows), falling back to the primary when none exists.
Generation itself is a one-shot forked child (basebackupPid, tracked the
same way service_archiver.c tracks pgReceivewalPid) rather than a
persistent service, so a potentially long-running pg_basebackup can't
stall the main loop's own node_active()/WAL-report tick. The child execs
the real, unmodified pg_basebackup client with --wal-method=none -- this
backup is deliberately not self-consistent on its own, since the
archiver's already-running WAL capture is what supplies the WAL needed
to reach consistency on replay -- then reads the resulting backup_label
for the authoritative start LSN/timeline and reports both start and
completion to the monitor via two new wrappers,
monitor_report_basebackup_started()/_completed() (monitor.c/.h), calling
the SQL functions M1's schema already shipped but nothing had called
yet. endlsn is best-effort: a live read of the source's current WAL (or
last-replayed, if the source is a standby) position right after the
backup finishes: not Postgres's own internal stop-backup LSN (not
observable from a plain CLI wrapper around pg_basebackup), but a
reasonable upper bound, and never fatal to the backup itself if that one
query fails.
Verified end-to-end against a real monitor + primary + archiver: the
bootstrap backup fires automatically, archiver_wal / basebackup rows
land correctly (source = 'live', status = 'complete', a real endlsn
distinct from startlsn), and the resulting directory passes
pg_verifybackup. Full SQL regression schedule (src/monitor, 20/20)
still passes.
Second half of Milestone 5 ("live first, then replay/volatile" per the
design doc's Build order). service_archiver_maybe_generate_basebackup()
now takes a bootstrap `live` backup as before, then -- on the very next
tick -- exercises the `replay`/`volatile` pipeline exactly once: extract
the last retained backup into a throwaway staging directory, point its
recovery at this archiver's own already-captured WAL (restore_command +
recovery.signal, entirely local, no network round trip), let it replay
forward and promote once it runs out of locally-captured segments, then
pg_basebackup it over loopback and discard the staging instance --
'volatile' means nothing survives between cycles.
Real frequency-driven scheduling (basebackup_policy's own frequency/
onpromotion/retention, resolved through get_archiver_policy()/
get_basebackup_policy()) is a deliberate follow-up, not built here: the
milestone-defining new capability is the replay mechanism itself, not a
general scheduler (matching the design doc's own build order, which
lists warm standby's scheduling machinery as a later milestone).
monitor_report_basebackup_started() (added in the `live`-only commit)
now takes real source/replaymode parameters instead of a hardcoded
'live', and monitor_get_latest_basebackup_location() is renamed to
monitor_get_latest_basebackup_info() and returns the latest backup's
source alongside its storage location -- what the trigger above uses to
tell "only the bootstrap has run" from "the replay exercise is already
done".
Getting a working staging instance up took two real, load-bearing fixes
along the way:
- pg_ctl start, invoked here through both a hand-rolled fork()/execl()
and this project's own run_program() helper, reproducibly misparsed
its own arguments in this exact process tree (deep in a supervised
archiver's own fork chain) even though byte-identical argv worked
fine in every standalone reproduction attempted. Root cause not
fully isolated; worked around by execing the real "postgres" binary
directly instead of going through pg_ctl at all -- the same
fork()/execv() pattern already used for pg_receivewal and
pg_basebackup in this codebase, with readiness confirmed by polling
a real SQL connection rather than relying on pg_ctl's own "-w".
- recovery_target_lsn set to "the end of the latest complete segment"
is not actually a reachable record boundary on a mostly-idle source
(a renamed, "complete" segment file is always its full fixed size
regardless of how much of it is real WAL) -- recovery correctly
refused to pause there ("recovery ended before configured recovery
target was reached"). Replaying to "everything locally available"
and letting Postgres promote on its own sidesteps needing a precise
target at all, which a `volatile`, discard-after-use snapshot never
actually needed in the first place.
Verified end-to-end against a real monitor + primary + archiver, from a
cold start through both the live bootstrap and the replay follow-up:
basebackup rows land correctly (source/replaymode/status all correct,
real distinct startlsn/endlsn across the sequence), and both resulting
directories pass pg_verifybackup. Full SQL regression schedule
(src/monitor) passed 20/20 twice earlier against this same unchanged
schema in this session; a later re-run hit an apparent local
pg_regress/DROP DATABASE environment hang (ProcSignalBarrier) unrelated
to any change in this commit -- no .sql files are touched here.
pgaftest coverage for Milestone 5's own base backup generation (both live and replay/volatile): brings up a monitor + primary + archiver, then waits for both the bootstrap live backup and the one-time replay/volatile follow-up to land, checking the group's final pgautofailover.get_latest_basebackup() row (source = 'replay', replaymode = 'volatile', status = 'complete'). No explicit trigger step is needed here, unlike archiver_wal_capture.pgaf's pg_switch_wal() calls -- both backups fire on their own within a couple of service_archiver_loop() ticks of the archiver starting. That also makes the intermediate 'live'-only state unsafe to assert on directly (this pass's own trigger logic produces at most one live and one replay backup before going quiet for the group, a couple of ticks apart, with nothing in this spec's control over exactly when to look) -- only the final state, once both have landed, is deterministic. Registered in tests/tap/schedule and tests/tap/schedules/node.sch, alongside archiver_wal_capture.pgaf.
New "Archiving & Disaster Recovery Architecture" section in intro.rst, between "Single Standby Architecture" and "Multiple Standby Architecture" -- an archiver is orthogonal to standby count, so it reads best as the thing you add on top of the simplest case before the doc branches into standby-count variations. New docs/tikz/arch-archiver.tex, rendered to .svg the same way every other architecture diagram in this directory is (latexmk -lualatex + pdftocairo, verified locally): primary + secondary + archiver, with the archiver's own WAL cache / base backups called out, a distinct WAL streaming (pg_receivewal) edge separate from real streaming replication, and the monitor's health-check/WAL-report edges to all three. common.tex gains one new color pair (abox/atxt, MS amber) and one new edge style (wal, dashed) for the archiver box and its WAL-streaming edge -- deliberately not reusing the primary/standby colors, since an archiver is a different kind of entity, not a replica. Terminology: uses "archiver" for the physical entity and "archiving node" for its per-group FSM membership, per-project decision -- avoids colliding with pgautofailover.archiver_node, the broader schema table that also covers warm-standby/pitr instances which don't participate in elections at all.
keeper->postgres.currentLSN was set to "0/0" once at service_archiver_ loop() startup and never updated again -- an archiving node's own reportedlsn in pgautofailover.node stayed at that placeholder forever, no matter how much WAL it had actually captured. This mattered more than it looked: pgautofailover.get_most_advanced_ standby() -- the query fast-forward uses to pick a WAL source during a failover election -- has no kind-based exclusion at all, and an archiving node already passes through REPORT_LSN_STATE during an election exactly like any other node (ARCHIVING_STATE -> REPORT_LSN_STATE, fsm.c). A "0/0" reportedlsn was the only thing keeping an archiver from ever being ranked as a candidate WAL source. service_archiver_update_current_lsn() now scans the local WAL cache for the newest complete segment each tick and updates currentLSN to its end LSN before keeper_node_active() reports it -- verified against a real cluster: after two pg_switch_wal() calls, the archiver's own pgautofailover.node.reportedlsn row tracks the primary's position almost exactly (0/A000000 vs. the primary's own 0/A000060).
…m an archiver
Confirms (and builds out) the reframing from the previous commit: an
archiving node already passes through REPORT_LSN_STATE during elections
and get_most_advanced_standby() has no kind-based exclusion, so once its
currentLSN is real, fast-forward's existing streaming-replication code
path can already select and target one -- no new restore_command
plumbing needed. Four real gaps stood between that and actually working,
found and fixed by testing a genuine, unmodified Postgres standby against
a real archiver end to end (not just pg_receivewal, which never exercises
any of these):
- pg_walsender routing is dbname-based (formation/group as dbname), but a
real standby's own walreceiver never forwards the operator's dbname for
a physical replication connection -- it always sends the literal
"replication", confirmed against a real standby. accept_loop.c now
falls back to the single configured route when it sees that sentinel,
matching this milestone's own one-membership-per-archiver scope; a
multi-route archiver (later milestone) needs a different mechanism
(e.g. application_name, which real walreceiver does forward).
- IDENTIFY_SYSTEM's systemid always fell back to the "unknown" placeholder
"0" because nothing ever populated route->systemId: service_archiver_
serve.c's own routes-file writer never wrote a systemid key, even
though routes.c already knew how to parse one. A real standby rejects
a mismatched system identifier outright ("database system identifier
differs between the primary and standby"). Fixed with a new monitor
RPC, monitor_get_group_system_identifier() (pgautofailover.
get_group_system_identifier(), new SQL function in both
pgautofailover.sql and the 2.2--2.3 migration -- an archiving node has
no sysidentifier of its own, but every other node in its group shares
the same one), wired into the routes-file refresh.
- cmd_start_replication.c read raw fread() bytes from a ".partial"
segment without knowing where pg_receivewal's actually-written data
ends -- pg_receivewal pre-allocates the full segment size up front
(matching real Postgres's own WAL file pre-allocation), so reading past
the real tail returns zeros indistinguishable from real content at the
byte level. Sending that tail as WAL data is exactly what a real
standby's recovery logic flags as "invalid record length ... got 0",
and on seeing it, terminates its own walreceiver outright rather than
treating it as "nothing new yet, retry" -- with no automatic
reconnection afterward. Fixed by trimming any trailing zero run before
ever sending a ".partial" chunk (self-correcting: an in-progress
boundary just gets re-read next tick instead of shipped early).
- get_most_advanced_standby() returns an ARCHIVING row's real nodeport,
which is the port == 0 sentinel (no postmaster of its own), not the
archiver's actual pg_walsender serve port -- the monitor has no column
for that (archiver-host-local information, same reasoning service_
archiver_serve.c's own routes file exists for). keeper_get_most_
advanced_standby() now resolves a port == 0 candidate to
PG_AUTOCTL_ARCHIVER_SERVE_PORT, matching this milestone's single-
well-known-port scope.
Verified end-to-end: a real pg_basebackup-seeded standby, given nothing
but an ordinary primary_conninfo pointing at the archiver's serve port,
completed backup recovery, reached consistent recovery state, streamed
live via START_REPLICATION, stayed connected indefinitely (pg_stat_wal_
receiver: status = streaming), and correctly applied newly-written data
(a table created and populated on the real primary afterward) -- with
zero restore_command, zero new replication-source machinery, and zero
changes to fsm_fast_forward's own selection logic beyond the port fix
above.
Bootstraps a brand new node from a registered archiver's base backup
plus captured WAL instead of the group's live primary -- the disaster-
recovery case: rebuild after every live standby (or even the primary)
is gone, with only the archiver left standing. Verified end to end
against a real cluster: `create postgres --from-archiver` completed
pg_basebackup from the archiver, replayed WAL, and settled into a
genuinely healthy "secondary" (pg_stat_wal_receiver: status =
streaming), matching reportedlsn against the real primary once it
re-parented there.
New plumbing:
- KeeperConfig.fromArchiver (keeper_config.h) plus the `--from-archiver`
CLI flag on `create postgres` (cli_create_node.c, cli_common.c) --
runtime-only, same as createAndRun, since reach_initial_state() runs
in the same `pg_autoctl create` invocation that parses it.
- pgautofailover.get_archiver_node() (pgautofailover.sql, the 2.2--2.3
migration) plus its monitor_get_archiver_node()/keeper_get_archiver_
node() C wrappers (monitor.c, keeper.c): finds the ARCHIVING row for
(formation, group) directly. Deliberately not get_most_advanced_
standby() -- that function filters on reportedstate = 'report_lsn', a
transient state an archiving node only visits during a FAST_FORWARD
election, never during its normal steady-state 'archiving' operation,
so it can never find an idle archiver outside of an election.
- fsm_init_standby() (fsm_transition.c) branches on config->fromArchiver
to resolve the archiver via the above instead of keeper_get_primary(),
and passes an empty replication slot name -- pg_walsender has no
slot-based retention in this milestone (cmd_start_replication.c's own
header comment), so standby_init_database's pre-flight replication-
slot check must be skipped rather than asked to verify a slot that
will never exist, matching that function's own existing "initialising
from another standby, no primary yet" precedent.
Four further real, narrow gaps stood between that and actually working,
each found by running the real `pg_basebackup`/`pg_autoctl` code paths
end to end rather than by inspection:
- pg_walsender's BASE_BACKUP had no manifest support (documented scope
cut, cmd_base_backup.c), but PG13+ pg_basebackup requests one by
default -- ReplicationSource.noManifest (pgsql.h) plus pg_basebackup()
passing --no-manifest when set (pgctl.c) works around it for an
archiver-sourced clone specifically, without touching a real primary's
own backup path.
- pgctl_identify_system() (pgctl.c) built its replication connection
string with no dbname at all, relying on real pg_basebackup's and
real walreceiver's own respective "default unset dbname to the literal
'replication'" behaviors -- neither of which this is: it's pg_auto_
failover's own raw libpq connection, which has no such default and
instead falls back to plain libpq's own "dbname = username" rule
(fe-connect.c), a route pg_walsender's routes file was never going to
have an entry for. Passing "replication" explicitly matches what every
other replication client already sends on the wire, and is a no-op
against a real primary (which ignores dbname for replication=true
connections regardless).
- A "replay" base backup (basebackup_replay_mode, milestone 5) promotes
a throwaway extracted copy to make it self-consistent, which genuinely
puts it on a *later* timeline than whatever the archiver's own walcache
has actually captured (which only ever advances on the real primary's
timeline) -- serving that pairing breaks a real pg_basebackup's own
timeline consistency check once it reaches its background WAL-streaming
step ("starting timeline N is not present in the server", comparing
the backup's own timeline against IDENTIFY_SYSTEM's). Fixed at the
source: pgautofailover.get_latest_basebackup() grew an optional
preferred_source filter (both SQL files), and service_archiver_serve.c's
routes refresh now asks for 'live' specifically -- a live-sourced
backup always shares the walcache's timeline by construction. A second,
independent, defense-in-depth check (walcache_current_timeline(),
comparing the walcache's own newest captured segment's embedded
timeline against whatever's about to be advertised) keeps the routes
file from ever serving a mismatched pairing even if that invariant is
ever violated by a future backup mode. monitor_get_latest_basebackup_
info() also grew a timeline out-param, threaded into the routes file's
own (previously unpopulated) "timeline" key -- already parsed by
routes.c, never written by anyone until now.
- cmd_start_replication.c ended a stream with bare CopyDone and nothing
else. A real, long-lived streaming client (real walreceiver, via
primary_conninfo) never triggers the gap because it never decides to
stop on its own -- which is exactly why this went unnoticed through
all of the earlier fast-forward-from-archiver verification. But
pg_basebackup's --wal-method=stream background WAL receiver does
decide to stop, once it reaches its own target LSN, and real receive-
log.c's ReceiveXlogStream only accepts that as a *successful* stop
when it can read a matching CommandComplete afterward (matching real
walsender.c's own WalSndDone, which sends exactly that on controlled
shutdown) -- without it, the client falls through to "unexpected
termination of replication stream" and exits non-zero even though
nothing was actually wrong on the wire. Fixed by sending a CommandComplete
tagged "COPY" right after CopyDone.
…iver Adds archiver_bootstrap_and_fast_forward.pgaf, the disaster-recovery scenario this whole investigation was driven by: a primary, an archiver, and a secondary that's created via `pg_autoctl create postgres --from-archiver` (not from the live primary) after the archiver's first live base backup is ready, then a FAST_FORWARD election where the archiver is the only node with the WAL the winning candidate is missing. node2 is declared `create and launch deferred`: the normal ini-driven node bring-up (`pg_autoctl node start`) has no hook for a custom flag like --from-archiver (NodeSpec/nodespec.c carries no such field -- fromArchiver lives only in KeeperConfig, populated exclusively by cli_create_node.c's own direct CLI parsing), so test_001 `exec`s into node2's own container and runs `pg_autoctl create postgres --from-archiver` by hand, then backgrounds `pg_autoctl run` the same way debug_citus_worker_switchover.pgaf backgrounds a long-lived process (`bash -c "nohup ... &"` -- a foreground `pg_autoctl run` would hang `docker compose exec -T` forever otherwise). test_002-004 engineer a real WAL gap rather than relying on race timing: stop node2 so it can't stream from node1 anymore, generate more WAL on the primary and give the archiver (still capturing independently via pg_receivewal) time to land it, kill the primary, then bring node2 back -- at that point the archiver is strictly ahead of node2 and is the only viable FAST_FORWARD WAL source. The final row-count check on node2 post-promotion confirms real WAL bytes were fetched and applied, not just that the FSM passed through the right state label. Verified: `pgaftest show spec`/`show compose` parse this spec cleanly (exit 0) and `pgaftest indent` round-trips it losslessly, confirming the DSL usage (deferred node declaration, exec/nohup backgrounding, multi-state `passing through` clause) is syntactically valid against the real grammar. Could not run it against a live docker compose cluster in this session: `make -f Makefile.docker build-pg17` fails fetching ghcr.io/hapostgres/pg_auto_failover/pgaf-base (401 Unauthorized, no registry credentials available here), and no local base image is cached to build from instead. Every C-level behavior this spec exercises (--from-archiver's own bootstrap, and fast-forward sourcing WAL from an archiver) was independently verified working end-to-end by hand against a real cluster in the two preceding commits on this branch.
wal_archived() is a plain LANGUAGE sql function (not SECURITY DEFINER), so it runs under the caller's own privileges. autoctl_node never got a direct SELECT grant on pgautofailover.archiver_wal: the blanket `GRANT SELECT ON ALL TABLES IN SCHEMA pgautofailover TO autoctl_node` only covers tables that already existed when that statement ran, and archiver_wal (like every other table in the M1 archiving schema) was created after it. Confirmed live: calling wal_archived() as autoctl_node (the role node_active() actually uses) failed with "permission denied for table archiver_wal". get_latest_basebackup() had this exact same bug, already fixed the same way (SECURITY DEFINER) in a prior commit -- apply the same fix here.
BuildForPrimaryNodeNodeActiveContext() counted every other node in the group toward replicationQuorumCount/secondaryNodesCount/ secondaryQuorumNodesCount, including ARCHIVING rows -- which are never real Postgres secondaries and can never report SECONDARY. In a formation with only a primary and an archiver, that miscount let the archiver's own bootstrap WAIT_STANDBY reading trip anyOtherNodeWaitingStandby (pos 401) and bump the primary off SINGLE, while secondaryQuorumNodesCount could then never legitimately reach zero -- so the primary got stuck between SINGLE and PRIMARY forever. Skip ARCHIVING (hasPgData=false) rows in that loop, matching the hasPgData-based exclusion this file's own REPORTING_NODE section already applies for a different purpose. Also adds the archiver-mirror FSM rows (pos 394/396/399) their own SINGLE|WAIT_PRIMARY|JOIN_PRIMARY match set, since a primary attached only to an archiver legitimately stays SINGLE the whole time instead of ever reaching WAIT_PRIMARY.
…(M5)
Several pieces of the Archiving & Disaster Recovery milestone, landing
together since they build on each other:
WAL-capture reliability
- service_archiver_start_pgreceivewal() now creates a replication slot
for pg_receivewal (pgautofailover_standby_<nodeId>), the same one
keeper_create_and_drop_replication_slots() already creates eagerly
on any primary for every other node regardless of kind. Without a
slot, a pg_receivewal that loses the startup HBA-propagation race
restarts from the server's then-current position, silently and
permanently skipping whatever WAL existed in between.
- pg_walsender's START_REPLICATION now fails loudly ("58P01") instead
of waiting forever when asked for a segment that predates this
archiver's own captured history and will never arrive.
- The archiver's real captured-WAL position is now tracked out of
band (a position file, service_archiver_position_path() and
friends) so it can cross the fork() boundary between the capture
and serve processes -- consumed by cmd_base_backup.c's own
end-of-backup position (previously could re-send a stale start
position and hang a real pg_basebackup's background WAL streamer
forever) and by cmd_identify_system.c indirectly via the routes
file's new "position" key.
- service_archiver_loop() now sets pgIsRunning = true for the
archiver's own keeper state, which the monitor's NodeIsHealthy()
unconditionally requires before ever selecting a node as a
FAST_FORWARD WAL source.
Telemetry
- service_archiver_report_storage() reports disk usage/free space to
the monitor periodically; monitor_get_archivers() surfaces it (and
each archiver's FSM state) to `pg_autoctl watch`'s new archivers
section.
Base-backup production/retention policy
- New SQL: get_basebackup_policy_for_group(), list_basebackups();
get_basebackup_policy() gains SECURITY DEFINER (needed now that
`pg_autoctl show basebackup-policy` calls it directly).
- service_archiver_basebackup.c's scheduling is now policy-driven
instead of the previous hardcoded "bootstrap live, then exactly one
replay, then quiet" scope: frequency/source/replaymode/onpromotion
read from whichever policy resolves for the group, plus
maxcount/maxage retention pruning after each successful backup. The
very first backup for a group is always sourced live regardless of
policy (nothing to replay from yet).
- The replay/volatile staging instance now starts with ssl = off:
the copied postgresql.conf/postgresql.auto.conf still carries the
source node's own ssl_cert_file/ssl_key_file paths, meaningless
here since the archiver has no Postgres SSL certs of its own --
left enabled, the staging instance failed outright at startup.
- New CLI: `pg_autoctl create/show/set basebackup-policy`, and
`pg_autoctl create archiver --basebackup-policy <name>` to attach
one at creation time via set_archiver_policy().
Verified via a full --no-cache Docker rebuild plus the archiver_wal_
capture, archiver_basebackup_generation, archiver_basebackup_policy,
and archiver_bootstrap_and_fast_forward pgaftest specs, all passing.
archiver_wal_capture.pgaf: fixed a wrong segment-1 assumption (the archiver's replication slot only protects WAL from its own creation time onward -- by the time it's created, node1+node2's own bootstrap has typically already consumed segments up to the empirically observed floor, segment 3) and switched two `wait until ... state is primary` assertions to the real terminal state after a permanent primary loss (`wait_primary`: WAIT_PRIMARY -> PRIMARY requires another node to reach reported SECONDARY, which an archiver never will). archiver_basebackup_generation.pgaf: the schema's own 'default' policy (frequency 24h) no longer produces a second, replay-sourced backup within any sane test window now that scheduling is policy-driven instead of hardcoded "bootstrap live, then exactly one replay". Attach a short-frequency, source=replay policy during setup so the spec's own remaining job -- proving the replay/volatile generation pipeline itself still works -- is still genuinely exercised. archiver_bootstrap_and_fast_forward.pgaf: same wait_primary fix as above, applied where this spec also stops the original primary for good partway through. New: archiver_basebackup_policy.pgaf, covering the base-backup policy feature end to end -- a fast-cycling, maxcount=3 policy created via the real CLI, attached via set_archiver_policy(), reaching and holding a stable retained count after several times its frequency has elapsed. Registered both archiver_basebackup_policy and (previously missing) archiver_bootstrap_and_fast_forward in tests/tap/schedules/node.sch. All four specs verified passing against a from-scratch --no-cache Docker rebuild.
Intro - Rewrote the opening paragraph: pg_auto_failover is a complete system (pg_autoctl runs as its own pid 1 supervising postmaster), not just an extension -- dynamic topology, automated or operator- driven, two modes of operation (command-driven CLI and node.ini + `pg_autoctl node run`). - New "High Availability, Disaster Recovery, and Backups: One System" section with a new two-panel diagram (arch-ha-dr-unified.tex/.svg) contrasting the typical separate-HA-tool/separate-backup-tool split against pg_auto_failover's single control plane for both. Failover State Machine - New "Archiving" subsection in the State reference, covering the ARCHIVING state's real transitions (verified against live `pg_autoctl inspect fsm list --json` output), its exclusion from candidacy/quorum, and its role as a Fast_forward-eligible WAL source. - Added the 3 real archiving edges to the "Node init / join" and "Failover / promotion" mermaid diagrams, with a new archiverState color class and cross-reference notes. Updated the "20 states and 77 transitions" summary line to 21/80. Fault Tolerance - New "Archiving Nodes and Disaster Recovery" section: WAL capture independent of any standby, base backups on a policy, rebuilding a node (or a whole formation) from an archiver's cache, and how archiving nodes participate in (and are excluded from) failover. Operations - New docs/archiving.rst page: registering an archiver, creating and attaching base-backup policies, watching an archiver, and rebuilding a node with `pg_autoctl create postgres --from-archiver` -- including the disaster-recovery case of rebuilding a whole formation from a single surviving archiver. Reference - New CLI reference pages for `pg_autoctl create/show/set basebackup-policy`, registered in their respective toctrees. Verified with a clean `sphinx-build -W --keep-going` (no warnings, no broken references).
…anels Replaces the single stacked arch-ha-dr-unified diagram with two separate figures, each a "production architecture" style pair of dashed service-boundary boxes with a header + inner service pills: - arch-ha-dr-typical: High Availability (Patroni, repmgr) next to Disaster Recovery + Backups (pgBackRest, pgBarman) -- two entirely separate boundaries, naming the actual products a typical setup reaches for. - arch-ha-dr-pgautofailover: High Availability + Disaster Recovery collapse into a single pg_auto_failover box; Backups (pgBackRest, pgBarman) remains its own separate boundary. Colors are a muted, readable palette local to these two diagrams (dark-tinted text, pale tints for fills) rather than raw saturated brand colors used directly as text -- the previous version's bright green header/body text (mbox, #9BF00B) was a real readability problem. Node heights are compact (1.35cm pills) instead of the previous 2.3cm/6.4cm boxes, since most of these boxes hold a single line of text. intro.rst's "High Availability, Disaster Recovery, and Backups: One System" section is retitled "High Availability and Disaster Recovery: One System" and its body adjusted to match: Backups, in the narrower sense of retention/cataloguing/cloud tiers, is now described as its own remaining concern rather than folded into "one system," matching what the new diagrams actually show.
…maid The five keeper-FSM mermaid diagrams had drifted from real KeeperFSM[] output -- verified by re-running `pg_autoctl inspect fsm mermaid <phase>` for all five phases and diffing byte-for-byte against what was checked into the docs. Real gaps found and fixed: - Failover / promotion was missing the entire "wherever you were, you're being demoted now" fan-out (init/single/catchingup/secondary/ prepare_promotion/stop_replication/maintenance/prepare_maintenance/ wait_maintenance/report_lsn/fast_forward, each with both a -> demoted and -> demote_timeout edge), plus several report_lsn fan-in edges (fast_forward/prepare_promotion/stop_replication/ demote_timeout/join_secondary -> report_lsn) -- 28 missing edges in this diagram alone. - Node removal / drop was missing wait_maintenance -> single and fast_forward -> single. - Maintenance was missing wait_maintenance -> report_lsn. - The archiving state's edges (added in an earlier, hand-written pass) are now the tool's own generated labels/coloring (electionState amber, not a separate hand-added archiverState class) instead of hand-embellished text not backed by any real KeeperFSM[] comment. Node init / join and Steady-state / config changes already matched exactly. Updated the summary line and the "replaces the old Graphviz diagram" note from the stale 80/68 transition counts to the real total: 21 states, 102 transitions (111 raw KeeperFSM[] edges minus the 9 excluded join_primary ones). Fixed the Failover / promotion intro paragraph's "still less than half the size of the full graph" claim -- at 57 of 102 edges it's now over half, which the added fan-out edges explain (most of that diagram's size is exactly that "interrupted from anywhere" fan-out). Added an explicit `archiving_state` label on the State reference's Archiving entry so other pages can :ref: it directly instead of relying on an implicit, same-document-only section-title link.
New docs/archiving-internals.rst, in the Architecture toctree: the technical reference for how archiving is actually built, meant to be the main place to extend for later milestones (warm standby, PITR, cloud push). Covers, grounded directly in the current source (function names, exact invocations, exact file paths): - The two forked processes per archiver (capture, serve) and the two files (archiver-position, archiver-routes.ini) that are their only channel to each other -- new arch-archiver-internals diagram. - WAL capture: how an archiver's replication slot reuses the exact same mechanism a real standby's slot uses, with zero primary-side special-casing; the exact pg_receivewal invocation; how the real captured position is computed (including .partial-segment trailing- zero trimming) and shared across the fork boundary; what happens to pg_receivewal across a failover. - Base backup generation: the basebackup_policy table and its 3-tier resolution chain; exactly when a backup is due (bootstrap, onpromotion, frequency); the live pg_basebackup invocation; the full replay/volatile pipeline (staging instance, recovery config, promote, basebackup over loopback, discard); retention pruning. - pg_walsender: why it's a from-scratch reimplementation (no frontend-linkable server-side replication library exists), its process model, the routes-file-based auth/routing mechanism, and a full table of every wire command it implements. - How pg_autoctl create postgres --from-archiver and FAST_FORWARD reuse the port==0 archiver-serve-port resolution trick to talk to pg_walsender with no archiver-specific code past that one lookup. - Build/process wiring, and an explicit "extension points" section listing what M6/M7/M8 build on top of, and what's schema-complete but not yet enforced (concurrency, allowed_hosts). Verified clean with sphinx-build -W --keep-going (no warnings, no broken references).
Mermaid diagrams already get pan/scroll-to-zoom via mermaid_d3_zoom (conf.py), but that's specific to Mermaid's own inline-SVG rendering and never applied to the tikz-rendered figures the rest of the docs embed via `.. figure::` -- those render as plain <img src="....svg">, which d3-zoom can't attach to. New docs/_static/js/zoom.js + css/zoom.css: a small, dependency-free overlay wired to every `figure img` at page load. Click (or Enter/Space when focused) opens the image full-screen on a dark backdrop; scroll to zoom, drag to pan, double-click to reset, Esc/backdrop-click/close-button to dismiss. Wired site-wide via conf.py's existing add_css_file/ add_js_file setup() hook, the same mechanism already used for the project's custom CSS. Verified interactively: click opens the overlay, wheel/drag/dblclick/Esc all behave as expected, and a clean sphinx-build -W --keep-going.
New top-level section right after the page's own intro, before "The pg_auto_failover Monitor": frames High Availability as two distinct guarantees -- Service Availability (the Postgres service stays reachable, what the rest of this page/failover-state-machine.rst/ fault-tolerance.rst describe) and Disaster Recovery (the data survives even total loss of every node that ever held it, what archiving-internals.rst and the archiver covers) -- cross-referencing into both rather than duplicating either. Adds a page-level `fault_tolerance` label to fault-tolerance.rst (it had no explicit label of its own) so this new section can :ref: it directly.
Several archiver specs used a "sleep N seconds, then run one SQL
query, then assert" pattern to wait for an async condition (a WAL
segment archived, an archiver reaching a state, a base backup
landing) instead of actually polling. This caused real CI flakiness --
a fixed sleep either wastes time past a condition that was already
true, or isn't long enough under load and produces a flaky failure.
Adds a new CMD_WAIT_SQL command:
wait until sql <service> { SQL } is { value } [timeout Ns]
which polls exec_sql_on_service() every second until its (substring-
matched, same semantics as `expect { }`) output contains <value>, or
times out. This is the building block; three sugar forms cover the
repeated shapes found across the archiver specs, all lowering to
CMD_WAIT_SQL at parse time with no new runtime machinery:
wait until wal segment "<segment>" archived in <formation>/<group>
wait until archiver state is <state> in <formation>[/<group>]
wait until basebackup <source|status|replaymode> is <value> in <formation>/<group>
The archiver-state form exists because an ARCHIVING membership row's
nodename is always synthesized by archiver_add_formation() as
'archiver-<archiverid>-<groupid>', never the plain --name given at
create-archiver time -- the ordinary "wait until <node> state is
<state>" form (which matches on nodename = $1) can't see these rows
at all, let alone disambiguate more than one membership sharing the
same archiver.
Grammar changes regenerated via `make generate` (src/bin/pgaftest),
zero bison conflicts. docs/ref/pgaftest.rst documents all four forms.
Verified: full grammar round-trip via `pgaftest indent` on every new
form, a hand-written timeout-path spec confirms clean 5s failure (no
hang, no false pass), and end-to-end Docker/pgaftest runs across all
8 specs that use or sit next to this feature (see next commit for the
migration itself).
Replaces every "sleep N + sql + expect" call site that was polling for an async condition with the new wait-until-SQL forms (previous commit), across: archiver_wal_capture.pgaf archiver_multi_formation.pgaf citus_basic_operation.pgaf archiver_budget_architecture_regions.pgaf archiver_two_regions.pgaf archiver_basebackup_generation.pgaf archiver_bootstrap_and_fast_forward.pgaf archiver_basebackup_generation.pgaf's own fixed 120s sleep (added in an earlier commit as a stopgap for CI flakiness) is replaced outright by the new "wait until basebackup ... is ..." polling form, which is the real fix that stopgap was standing in for. archiver_basebackup_policy.pgaf is deliberately NOT migrated: its `count(*) = 3` check needs a stable, settled value after enough retention cycles have elapsed, not a first-reach-true poll -- a naive poll-until-true would risk a false pass on a transient count. Its existing fixed sleep is correct by design, not a flakiness bug. Also fixes a second, real bug this migration surfaced in archiver_bootstrap_and_fast_forward.pgaf's test_001: the monitor's own "basebackup complete" status and pg_walsender's actual ability to serve that backup are two different things. cmd_base_backup.c checks route->basebackupDir, which service_archiver_serve.c only refreshes every ARCHIVER_SERVE_ROUTES_REFRESH_TICKS (30) ticks -- so there's a real window where the monitor says "complete" before the archiver's own route is servable. The instant wait-until-SQL poll exposed this race (the old spec's blind sleep 30s happened to also absorb it by accident). There's no SQL-observable signal for "the archiver's route is ready", so this bridges the known 30s refresh window with a documented sleep rather than guessing at, or inventing new machinery for, something that isn't visible from the monitor side. Verified end-to-end via Docker/pgaftest: all 7 migrated specs pass in full, plus archiver_basebackup_policy.pgaf as an unmigrated regression check (2/2, unaffected). citus_basic_operation.pgaf runs its full 16-step Citus HA suite clean (~3.5 min). No C files touched by this commit; make docker-check / banned.h.sh are clean regardless.
Real PG19 libpq performs a "GREASE" self-test on every new connection
(borrowed from TLS): it deliberately requests a bogus minor protocol
version (major=3, minor=9999) plus a "_pq_.test_protocol_negotiation"
startup option, to verify the server negotiates down properly rather
than silently accepting whatever was asked. A server that accepts it
without negotiating is treated as broken and the connection is
refused: "server incorrectly accepted \"grease\" protocol version
3.9999 without negotiation" -- this broke every PG19 archiver
connection in CI (pgaftest / archiver (PG19), the exact "create
postgres --from-archiver" bootstrap path).
ws_startup_negotiate() only ever checked the major version
((code >> 16) != 3) and ignored the minor version entirely, so it
just proceeded with whatever was requested, including the grease
probe's own nonsense value.
Fixed with a real NegotiateProtocolVersion ('v') response, matching
Postgres's own backend behaviour:
- new ws_send_negotiate_protocol_version() (framing.c/.h) sends the
full encoded version (major<<16 | newest supported minor) followed
by a count and list of unrecognized "_pq_.*" startup options --
real libpq's own pqGetNegotiateProtocolVersion3() rejects a
response that isn't properly encoded as "downgrade to pre-3.0",
and separately requires any _pq_.* option the client sent to be
echoed back as unsupported (we don't parse any, so every one seen
is unsupported by definition).
- ws_startup_negotiate() now parses the startup packet's key/value
pairs before responding (needed to collect the _pq_.* option
names), and sends the negotiate message whenever the requested
minor version isn't 0 (all pg_walsender actually implements),
continuing the connection at that version rather than closing it.
Verified against real PG19 beta2 psql (which performs the same
GREASE probe as libpq) connecting directly to a standalone
pg_walsender: IDENTIFY_SYSTEM succeeds, no negotiation error. End to
end: archiver_bootstrap_and_fast_forward.pgaf passes all 4 steps on
PG19, including the exact bootstrap step that failed in CI. Full
regression pass (archiver_wal_capture, archiver_basebackup_generation,
archiver_basebackup_policy) on PG19 unaffected.
|
Hi @dimitri , I wanted to request, Is it possible to create a new release tag for pg_auto_failover? |
archiving-details.rst mentioned archiver-routes.ini in passing but
never explained when or why it gets rewritten -- "regenerated
automatically on the archiver's own next tick" was the full extent of
it. Adds a dedicated "Keeping the routes file current" section to the
Process model, covering:
- why pg_walsender never queries the monitor directly (staying
serve-capable through a monitor outage, staying a small
standalone/testable binary)
- the atomic write pattern (temp file + rename) and why it makes
concurrent reads inherently safe, no locking needed
- all four refresh triggers: startup, the 30s periodic tick,
immediate refresh on SIGUSR1 after a base backup completes (see
the next commit), and SIGHUP
- that multiple memberships' base backups can genuinely run
concurrently, with no archiver-wide serialization
Also includes a real, verbatim sample of the file's contents, and
tightens the Storage section's own brief mention to cross-reference
the new section instead of repeating a vaguer version of the same
explanation.
Verified: `make -C docs html` builds clean, no warnings, no unresolved
cross-references.
service_archiver_serve.c's routes file only refreshed on a blind 30s
timer (ARCHIVER_SERVE_ROUTES_REFRESH_TICKS), so there was a real
window where the monitor already reported a base backup "complete"
before pg_walsender's own route (route->basebackupDir, cmd_base_
backup.c) reflected it -- previously worked around in
archiver_bootstrap_and_fast_forward.pgaf with a blind 30s sleep.
Adds a dedicated SIGUSR1 signal so the capture process that just
finished generating and reporting a backup can prompt an immediate
refresh instead of waiting for the next tick:
- src/bin/common/signals.c/.h: new asked_to_refresh_routes flag,
wired to SIGUSR1 via catch_refresh_routes(), registered in the
shared set_signal_handlers() (installed everywhere, same as
asked_to_reload/SIGHUP, harmless where nothing checks it) and
added to block_signals()'s masked set.
- service_archiver_serve.c: service_archiver_serve_loop() checks
the flag every iteration, same pattern as the existing SIGHUP
check, and refreshes immediately when set.
- keeper_config.h: new archiverPidFilePath field, carrying the
archiver-level supervisor's own shared pidfile path (one "<pid>
<service name>" line per supervised service) across into a
per-membership KeeperConfig, whose own pathnames.pid gets
overwritten with a different value moments later.
- service_archiver_reconciler.c: build_membership_keeper() stashes
this path from the template keeper right after the shallow copy,
before the per-membership pathname recompute overwrites it.
- service_archiver_basebackup.c: new notify_archiver_serve_of_new_
basebackup(), called from the forked generation child right after
a successful backup. Looks up archiver-serve's own pid via
supervisor_find_service_pid() (the project's existing helper for
resolving one named service inside a shared multi-service
pidfile) and sends it SIGUSR1. Best-effort: any failure here
(pidfile missing, process already gone) is logged and otherwise
ignored -- archiver-serve's own periodic refresh is still the
fallback, and this must never turn an already-successful base
backup into a failure.
Caught mid-implementation: an earlier version of this used a plain
read_pidfile() on archiverPidFilePath, which only reads the first
line -- the supervisor's own pid, not archiver-serve's, since that
pidfile has one line per supervised service. Confirmed via live
Docker testing: the signal reached the supervisor (which has the
handler installed everywhere, per the above) but nothing ever ran
service_archiver_serve_loop() in that process, so the flag was set in
the wrong process's memory and routes.ini stayed on the 30s tick the
whole time (observed once at 117s -- 4x the tick, not immediate).
Fixed by switching to supervisor_find_service_pid(), which is what
this project already uses elsewhere to look a specific service up by
name in exactly this pidfile format.
Also removes the archiver_bootstrap_and_fast_forward.pgaf bridging
sleep this was meant to replace, and updates that step's own header
comment accordingly.
Verification status: local build (zero warnings), citus_indent, and
banned.h.sh are all clean. The corrected pid-lookup logic was traced
carefully against supervisor_find_service_pid()'s own implementation
and the pidfile format it expects, and a live Docker pass confirmed
the mechanism doesn't crash or hang archiver-serve and that concurrent
backups on different memberships both land correctly in routes.ini
with no corruption. A full, clean, uncontaminated end-to-end timing
proof (routes.ini updating within ~1s of a backup completing, not up
to 30s later) was attempted but not obtained in this session --
repeated verification passes hit infrastructure issues (a Docker
Compose project-name collision between two concurrent local
investigations, then agent/watchdog stalls on image rebuilds) rather
than any observed test failure after the pid-lookup fix landed. Worth
a clean, solo re-verification pass before relying on this further.
routes.ini used to carry everything pg_walsender needed -- walcache dir,
latest basebackup dir, timeline, position, systemid -- rebuilt from a
monitor query by archiver-serve on a schedule (startup, every 30s,
SIGHUP, and a SIGUSR1 push after each base backup). That push mechanism
closed most of the staleness window it was built to fix, but not all of
it: correctness still depended on a signal reaching a specific *live*
process, with no way for a reader to detect a stale value if it hadn't.
Replaces it with facts written exactly once, by whichever process is the
sole owner of that fact, at the moment it becomes true -- there is
nothing left to periodically refresh or push an update about, because
nothing is a cached copy of something else:
- archiver-routes.ini shrinks to a pure mapping: one [formation/group]
section, "path = <local storage root>", nothing else. Ownership
moves from archiver-serve to the reconciler (service_archiver_
reconciler.c) -- it's already the process that discovers a
membership's addition or removal, the only two moments this mapping
actually changes, so it writes the file exactly then (plus once at
its own startup) instead of on a schedule.
- archiver-systemid: new, one per membership, written once by the
capture process (service_archiver.c) the first time the monitor
reports it. Never rewritten after that -- a system identifier is
set at initdb and immutable for a cluster's lifetime, so write-once
isn't a simplification, it's the actually-correct behavior.
- basebackups/.latest: new, a one-line pointer to the current live-
sourced backup's label, written atomically by the base-backup
generation child (service_archiver_basebackup.c) the instant it
knows the backup is complete -- the same process, same moment, the
monitor is told. Retention pruning clears the pointer if the backup
it names is the one being pruned. Never written for a replay-sourced
backup.
- pg_walsender's BASE_BACKUP handler (cmd_base_backup.c) resolves the
backup directory from .latest and re-derives start LSN/timeline from
the backup's own real backup_label file (already true before this
change) and end LSN by scanning the WAL cache directly (previously a
fallback, now the only path -- the old routes.ini "position" field
was never anything but a cache of the same scan). Also adds a read-
time timeline-compatibility check (backup's own timeline vs the WAL
cache's current one, both already being computed), replacing an
equivalent check archiver-serve used to do once at write time --
checking fresh at read time catches drift a periodic write-time
check can't (e.g. the WAL cache advancing past a failover between
base backups).
- cmd_identify_system.c reads archiver-systemid directly instead of a
routes.ini field.
- archiver-serve (service_archiver_serve.c) no longer talks to the
monitor at all -- it only execs pg_walsender and supervises its
liveness. This finally delivers this milestone's own original design
goal in full: pg_walsender (and now its supervisor too) keeps
serving already-captured data through a monitor outage with nothing
in the serving path depending on the monitor being reachable.
Removed as dead weight once nothing calls it anymore: the SIGUSR1/
asked_to_refresh_routes signal plumbing (signals.c/.h), archiverPidFilePath
(KeeperConfig), and archiver-serve's own routes-building functions
(service_archiver_serve_refresh_routes/_write_route/_membership_config,
walcache_current_timeline).
Docs: archiving-details.rst's "Keeping the routes file current" section
rewritten as "Keeping local files current" describing the new ownership;
Storage section's directory tree and bullet list updated for the two new
files; the arch-archiver-internals tikz diagram regenerated to show the
reconciler (not serve) writing archiver-routes.ini and pg_walsender
reading archiver-systemid/the WAL cache directly. archiver_bootstrap_and_
fast_forward.pgaf's own header comment updated to match.
Verification: full local build clean (pg_autoctl, pg_walsender,
pgaftest, zero warnings under -Wall -Werror), citus_indent clean, Sphinx
docs build clean. Docker/pgaftest, rebuilt PG17 image against this
branch: archiver_bootstrap_and_fast_forward.pgaf (4/4, including the
BASE_BACKUP bootstrap through basebackups/.latest + archiver-systemid,
and a FAST_FORWARD WAL fetch through pg_walsender's other commands),
archiver_basebackup_generation.pgaf (1/1), archiver_wal_capture.pgaf
(3/3), archiver_multi_formation.pgaf (5/5, including the reconciler's
dynamic add-membership path writing archiver-routes.ini) -- 13/13 test
steps passed.
…rift
Three independent fixes plus a documentation pass, all found while
chasing down CI failures on the pgaftest / archiver jobs:
* FAST_FORWARD hang (all PG15-19 archiver jobs, test_004): a node
rebuilding from an archiver could hang forever converging on a
fast-forward target. service_archiver_update_current_lsn() preferred
the current ".partial" segment's raw captured-byte-count over the last
*complete* segment when reporting this archiver's currentLSN -- a raw
byte count isn't guaranteed to land on a genuine WAL record boundary,
and Postgres's own replay can only ever advance to one. Once the
source primary is gone, a target that isn't a real boundary is never
reached. Fixed by only ever reporting complete-segment boundaries;
pg_walsender's own streaming still serves everything available,
including ".partial" content, so replay always has real data to
advance through and past the (now-reachable) target.
* PG14-only BASE_BACKUP breakage (test_001, all PG14 archiver jobs), two
separate bugs stacked on top of each other:
- cmd_base_backup.c unconditionally used PG15+'s typed/multiplexed
tar-archive framing ('n'/'d' tagged CopyData messages), which a
pre-15 pg_basebackup client's receiving code has no path for at
all -- it predates that framing entirely. Now gated at compile
time on PG_VERSION_NUM (pg_walsender is built once per PGVERSION
against that version's own headers, so this is already the
archived group's real major version).
- tar_stream_directory() also sent a standalone-tar-file trailing
two-zero-block end-of-archive marker that real Postgres never
puts on the wire for a client-streamed, WAL-not-included backup
(confirmed against PG14's own perform_base_backup(), which sends
CopyDone right after the last file's content -- no terminator).
A pre-15 client's simpler receiving state machine takes the
"next CopyData between files" literally and requires it to be
exactly one tar block, erroring out on this extra chunk. Removed;
never needed by any client this project serves.
* Base-backup-vs-WAL-capture bootstrap race: a membership's first live
base backup and its WAL-capture child are two independent processes
with no ordering dependency between them. Generating that first
backup before the capture child's replication connection is even
established can report a start LSN older than anything the archiver
will ever capture, later surfacing as a background WAL streamer
erroring out ("requested WAL segment predates this archiver's
captured history"). service_archiver_maybe_generate_basebackup() now
holds back the bootstrap backup until the walcache has real WAL data.
* archiver-routes.ini relocated from an XDG-derived path (a sibling of
pg_autoctl's own config file, outside PGDATA, requiring config.c logic
pg_walsender never links) to <pgdata>/archiver-routes.ini. pg_walsender
now takes --pgdata (defaulting to $PGDATA) and derives the routes path
itself; --routes is gone.
* Doc fixes from a review pass: nonexistent `pg_autoctl archiver run`
command references corrected to `pg_autoctl run`/`pg_autoctl node run`;
pg_basebackup added to the process-tree diagrams (it was missing);
the WAL-position paragraph rewritten to describe the archiver-position
cache instead of claiming an always-cheap directory scan; "What you can
point at an archiver" renamed to "Archiving: client & server"; new man
pages for `pg_autoctl archiver serve` and `pg_walsender`.
Verified: citus_indent clean; full local build; all 7 archiver .pgaf
specs pass on PG17; archiver_bootstrap_and_fast_forward.pgaf and
archiver_basebackup_generation.pgaf (the two PG14-specific failures)
each run clean 3/3 on PG14 after rebuilding both Docker images.
service_archiver_basebackup.c queried a base backup's source node (live) or
its own staging instance (replay) for the current WAL position right after
each backup completed, using a hardcoded dbname=postgres connection string.
pg_auto_failover's own pg_hba.conf rules for a node are always scoped to
exactly that formation's real dbname (and "replication"), never "postgres"
unless a formation genuinely was created with that dbname -- confirmed live
against this project's own test images, where the "default" formation's own
dbname is "demo", not "postgres".
Live-source queries degraded silently (falls back to reporting startLsn
instead of a real end-LSN) so no test failed because of it, but it was a
real, always-reproducing inaccuracy for every live base backup, on every PG
version, the whole time.
Added monitor_get_formation_dbname() (monitor.c/h), modeled directly on the
existing monitor_get_formation_number_sync_standbys() pattern, and a small
fetch_formation_dbname() helper that looks it up once per backup and falls
back to the previous DEFAULT_DATABASE_NAME guess on any monitor-lookup
failure, so a transient monitor outage degrades back to prior behavior
rather than turning this into a new fatal failure mode.
Verified: citus_indent clean; full local build; confirmed via direct SQL
that this project's own archiver test topology already uses a non-default
formation dbname ("demo"); ran archiver_basebackup_generation.pgaf against
a freshly rebuilt image and confirmed the "no pg_hba.conf entry ... database
\"postgres\"" warning that used to appear during the live WAL-position query
is gone, with no new "Failed to retrieve dbname" fallback warning either
(the monitor lookup succeeds and the real dbname is used). All 7 archiver
.pgaf specs still pass cleanly.
CI's Style check job failed on ci/banned.h.sh: main.c's own --pgdata/PGDATA handling (added in e031a9f, the archiver-routes.ini relocation) called getenv("PGDATA") directly, which is on this project's banned-API list (caught locally too: sh ci/banned.h.sh now passes clean). Replaced with env_utils.c's own get_env_pgdata(), the project's established wrapper for exactly this -- already linked into pg_walsender via src/bin/common/'s wildcarded source list, no build changes needed. Verified: sh ci/banned.h.sh passes; citus_indent clean; full local build; archiver_wal_capture.pgaf and archiver_bootstrap_and_fast_forward.pgaf both pass clean against a freshly rebuilt image (get_env_pgdata() behaves identically to the removed getenv() call when PGDATA is set, as it always is in these containers).
Contributor
Author
Hi @RamaTripathi ; I would like to finish a couple features (this one included, where I believe there are 3 PRs on-top of this one coming) and also fix most of the remaining issues before a release, so that will push to October or something. For more influence, and because the project has zero financial support at this time, please consider having a look at my own sponsoring program at https://oss.theartofpostgresql.com. |
create_archiver_node()'s maxresidentreplay check and report_basebackup_ started()'s own bookkeeping both used an unlocked count-then-insert pattern, racy under concurrent calls for the same archiver. The monitor's own concurrency convention (LockFormation/LockNodeGroup, metadata.c) is a C-level advisory lock with no SQL-callable production wrapper (testing_lock_formation is testing-only), so both functions now take a row lock on the parent archiver row via SELECT ... FOR UPDATE instead -- the plpgsql-layer equivalent of the same "lock the parent before checking a derived constraint" principle. basebackup_policy.concurrency was read but never enforced. Since report_basebackup_started() is only called after pg_basebackup already completed (it reads the real start LSN from the finished backup's backup_label), gating there alone would only reject work after the expensive part already ran. Added a pre-flight pgautofailover.basebackup_concurrency_available() check, called from service_archiver_maybe_generate_basebackup() before starting the job -- skips the tick (never errors out the cycle) when the cap is full, matching the design's own "a queued job waits, it never skips" intent. The row-locked report_basebackup_started() check remains the atomic backstop for the race the unlocked pre-flight read can't fully close. Regenerated archiving_schema.out fixtures (default + pg19 override): the FOR UPDATE comment block shifted create_archiver_node's RAISE line number from 18 to 26; no behavior change to the assertions themselves. Verified: sh ci/banned.h.sh and citus_indent --check both clean; full regression + isolation suites pass against real PG17 and PG19 (20/20 + 6/6 each, via pg_virtualenv); pgaftest archiver specs pass against PG17 (archiver_wal_capture, archiver_basebackup_generation, archiver_bootstrap_and_fast_forward, archiver_basebackup_policy, archiver_two_regions, archiver_budget_architecture_regions -- archiver_multi_formation blocked locally by an unrelated Docker network-pool conflict with other containers on this machine, not retested).
service_archiver_start_pgreceivewal() built the archiver's connection to the primary as a hardcoded trust/no-password key=value string -- it never went through this codebase's own SSL/password conninfo builder, so an archiver could never join a cluster running with anything other than trust auth. Fixed by reusing prepare_primary_conninfo() (pgctl.c), the exact helper every ordinary standby's own primary_conninfo already goes through: exported it (was static) and call it from service_archiver.c with config->pgSetup.ssl and config->replication_password, so sslmode/sslrootcert/sslcrl and password= are set exactly the way they are for any other node kind. Cert authentication needs no extra wiring beyond sslmode itself -- libpq already discovers the client certificate from ~/.postgresql/ once SSL is requested, the same way every other node in a cert-auth cluster does. Neither the CLI nor the declarative node.ini path had anywhere to supply these before: - cli_create_archiver_getopts (cli_create_node.c) gains --replication-password and the same --ssl-self-signed/--ssl-mode/ --ssl-ca-file/--ssl-crl-file/--server-cert/--server-key/--no-ssl flags every other node kind already has -- but, unlike cli_create_node_getopts, does NOT require an explicit SSL choice: an archiver created with no SSL flag at all keeps the exact trust/no-password conninfo this milestone originally shipped with. - nodespec_build_create_argv's own archiver branch (nodespec.c) never passed any SSL/password flag through at all -- explicitly documented as deliberate at the time, since cli_create_archiver_getopts had nowhere to put them. Now mirrors the ordinary-node path's own ssl/replication_password argv construction, so `pg_autoctl node run` against a node.ini with kind = archiver picks up the cluster's ssl/auth settings the same way every other node kind's ini already does -- no format change to node.ini itself, compose_gen.c already writes [ssl]/[options]/[replication] generically for every node kind. docs/ref/pg_autoctl_create_archiver.rst updated to match. Test coverage: tests/tap/specs/ssl_cert.pgaf (verify-ca + cert auth, previously node1/node2 only) gains archiver1, inheriting the cluster's ssl/auth settings with no per-node override, plus test_005_archiver_cert_auth_wal_capture: confirms archiver1's own ssl.sslmode is verify-ca, then proves pg_receivewal actually authenticated (not just configured) by writing to the post-failover primary and watching the ARCHIVING node's reportedlsn advance past 0/0 -- the same proof-of-capture pattern citus_basic_operation.pgaf's own archiver coverage uses. Verified: sh ci/banned.h.sh and citus_indent --check both clean; full build (monitor + bin, including pg_walsender) against real PG17 in docker, `pg_autoctl create archiver --help` shows the new flags; pgaftest specs pass against PG17: ssl_cert (6/6, including the new step), ssl_self_signed (5/5, unaffected -- no archiver in that spec), archiver_wal_capture (3/3), archiver_basebackup_generation (1/1), archiver_bootstrap_and_fast_forward (4/4) -- the trust-auth archiver path is unchanged when no SSL/password flag is given.
archiving-details.rst's "Network exposure" section claimed the archiver's connections carry "no password or TLS ... in the current release" -- true only for the inbound side (client -> pg_walsender, still trust-only/no-TLS, unchanged), no longer true for the outbound side (archiver -> primary, service_archiver.c's pg_receivewal conninfo) since it now goes through prepare_primary_conninfo() and supports the same SSL/cert/password auth any other node's own primary_conninfo does. Split the section into the two connections explicitly rather than leaving one ambiguous claim covering both. pg_autoctl_node.rst's node.ini reference documented [ssl]/[options] ssl/[replication] replication_password purely in terms of an ordinary node's own live-reloadable Postgres SSL config (`pg_autoctl enable ssl`) -- true for every other kind, but for kind = archiver these same ini keys now drive the archiver's own outbound conninfo instead (nodespec.c's archiver branch, wired in the prior commit), immutable and create-time only, no live "enable ssl" equivalent for an archiver. Added the same kind of "for kind = archiver, this means something different" note the [formation] section already uses for its own per-kind difference. No code changes; doc-only follow-up to be9971e.
…ss code
Two concrete instances where new code duplicated an existing facility
instead of calling it, found in review:
1. run_pg_basebackup() (service_archiver_basebackup.c) hand-rolled its
own fork()+execv()+waitpid() to invoke pg_basebackup, right next to
copy_directory_tree() in the same file -- a function whose own
comment explains it uses run_program() because that "matches every
other external-program call in this codebase". run_pg_basebackup()
didn't follow its own neighbor's stated principle: no output capture
on failure (just a generic "pg_basebackup failed" with nothing from
the command's own stderr), no command-line logging. Rewritten to use
run_program() the same way, and to build its conninfo through
prepare_primary_conninfo() (pgctl.c) instead of a hardcoded
--no-password/no-SSL arg list -- the same sslmode/sslrootcert/
password support service_archiver_start_pgreceivewal() already has.
pgctl.c's own pre-existing pg_basebackup() was deliberately NOT
reused directly: that function's whole point is standby init --
it rmtree()s the destination pgdata and moves the finished backup in
its place, becoming the caller's new PGDATA. Calling it unmodified
here would replace this archiver's own WAL-cache root with backup
content, which is actively wrong for this use case (the artifact
needs to land in backupDir as an independent copy while the archiver
keeps running against its own, untouched pgdata).
Caught in testing: run_pg_basebackup() now takes an explicit
SSLOptions parameter rather than reading config->pgSetup.ssl
unconditionally. That field is this archiver's policy for connecting
to real, externally-managed cluster nodes (the primary, a live
standby) -- correct for generate_live_basebackup()'s source, wrong
for generate_replay_basebackup()'s loopback connection to its own
throwaway staging instance (a bare extracted copy with no working
SSL of its own). Reusing the archiver's own SSL policy there made
archiver_basebackup_generation.pgaf fail outright ("server does not
support SSL, but SSL was required") instead of degrading gracefully
the way the old hardcoded conninfo did (no sslmode specified at all,
libpq's own "prefer" default). Fixed by passing a zero-value
SSLOptions for the staging connection specifically.
2. service_archiver_serve.c hand-rolled its own inner fork()+execv()+
waitpid(WNOHANG) polling loop to supervise pg_walsender, nested
inside a process that was ALREADY a supervisor.c Service itself --
two layers doing the same "start it, notice it died, restart it"
job. Collapsed into one: service_archiver_walsender_start() is now
registered directly as a supervisor.c Service (matching
service_postgres_start()'s own fork-then-execv shape for the real
Postgres child), so supervisor.c's existing generic tick loop
handles liveness/restart/shutdown the same way it already does for
every other permanent service in this project. Removed
service_archiver_serve_loop()/_start_walsender()/_stop_walsender()/
_walsender_is_running() entirely -- ~120 lines of duplicated
supervision logic. `pg_autoctl archiver serve` (the standalone
command) now calls supervisor_start() with a single-Service array
instead of the old bespoke loop, for identical behavior.
Investigated but NOT changed: pg_receivewal's own liveness handling
(service_archiver.c's service_archiver_loop(), the FSM-tick-based
check referenced in that file's header comment). Confirmed this one is
NOT a bug -- service_archiver_loop() already checks
service_archiver_pgreceivewal_is_running() and restarts it on every
tick while in ARCHIVING_STATE, re-resolving the current primary fresh
each time; archiver_wal_capture.pgaf's own test_002_archiver_restart_
liveness exercises this path and passes. (An earlier round of this
review mischaracterized this as broken, based on keeper.c's generic
keeper_ensure_current_state() lacking an ARCHIVING_STATE case -- that
function is never reached for archiver nodes at all, since cli_
service.c branches straight to start_archiver() for nodeKind =
archiver, bypassing service_keeper.c's ordinary node-active loop
entirely. Correcting that here.)
Left as a real, working design choice rather than refactored: unlike
pg_walsender, pg_receivewal's supervision is coupled to FSM state
(only runs during ARCHIVING_STATE, restarts with a freshly-resolved
primary on each check) in a way a bare RP_PERMANENT supervisor.c
Service doesn't directly express. This is architecturally the same
shape as why service_postgres_ctl.c also doesn't use a plain
supervisor.c Service for the real Postgres process: postgres needs to
be actively stopped in some states (DEMOTED/DRAINING) and left
entirely alone for the operator in others (MAINTENANCE), a policy no
fixed RestartPolicy can express. pg_receivewal doesn't have an
equivalent "hand it to the operator" state today, so it's a real
migration candidate for a future pass (reconciler.c already proves
dynamic Service registration works for exactly this granularity) --
just not bundled into this change given the risk of altering a
currently-correct, tested mechanism without a dedicated pass.
Verified: sh ci/banned.h.sh and citus_indent --check both clean; full
build (monitor + bin, pg_walsender included) against real PG17 in
docker; pgaftest specs pass against PG17: archiver_wal_capture (3/3,
including the restart-liveness test), archiver_basebackup_generation
(1/1, the regression this change introduced-then-fixed),
archiver_bootstrap_and_fast_forward (4/4), archiver_basebackup_policy
(2/2), ssl_cert (6/6, including the cert-auth WAL-capture step from
the prior commit).
pg_basebackup() bundled two genuinely separate concerns in the middle of one function: (1) actually running pg_basebackup and reporting success/failure, and (2) standby init's own "the result becomes the caller's new PGDATA" rmtree-and-move ending. Every non-standby-init caller that wants a base backup as an independent artifact (this archiver's own base-backup production, service_archiver_basebackup.c) had no way to get (1) without (2) -- exactly why run_pg_basebackup() hand-rolled its own separate conninfo-building-and-subprocess-running copy instead, two functions with a common piece of code in the middle of them, per this repo's own review. Split into: - pg_basebackup_fetch(pg_ctl, replicationSource): runs pg_basebackup into replicationSource->backupDir and nothing else -- the actual mechanics (conninfo, PGPASSWORD dance, args, execute_subprogram, error reporting), unchanged from before the split. - pg_basebackup(pgdata, pg_ctl, replicationSource): now just ensure_empty_tablespace_dirs() + pg_basebackup_fetch() + the rmtree-and-move ending -- standby init's own exclusive use case, unaffected in behavior. ReplicationSource gains two optional fields, both empty-by-default (every existing caller's behavior unchanged): walMethod (empty means "stream", pg_basebackup()'s own long-standing default) and label (empty means no --label, as before). service_archiver_basebackup.c's run_pg_basebackup() now populates a ReplicationSource (walMethod = "none", label = the backup's name) and calls pg_basebackup_fetch() directly -- no more separate run_program()-based implementation duplicating the same conninfo/subprocess-launch logic pgctl.c already had. Verified: sh ci/banned.h.sh and citus_indent --check both clean; full build against real PG17 in docker; pgaftest specs pass against PG17: basic_operation (28/28, exercises ordinary standby init through the now-split pg_basebackup() -- the regression-risk case), archiver_wal_ capture (3/3), archiver_basebackup_generation (1/1, exercises the new pg_basebackup_fetch() path directly), archiver_bootstrap_and_fast_ forward (4/4, exercises create postgres --from-archiver, another pg_basebackup()-adjacent path).
Every existing caller of supervisor_start()/supervisor_start_with_
callback() passes a fixed-size, pre-populated services[] array, so
pendingSubprocessCount is > 0 from the very first check. That's the
only case supervisor_loop() was ever written for: its own
`while (pendingSubprocessCount > 0)` skips the loop body entirely
when a caller legitimately starts with zero services -- nothing to
supervise yet, added later via supervisor_add_service() from within
a periodicCallback (Supervisor.periodicCallback, supervisor.h). With
the loop body never running, supervisor->exitMode is never set away
from its zero-initialized SUPERVISOR_EXIT_ERROR, so the whole call
fails immediately with "Something went wrong in sub-process
supervision" before the callback ever gets a chance to add anything.
service_archiver_reconciler.c's own calloc(serviceCount > 0 ?
serviceCount : 1, ...) shows the zero-service start was anticipated
-- it just happens that every archiver config today always has >= 1
formation attached at creation time, so this path had never actually
been exercised.
Fixed by keeping the loop running past pendingSubprocessCount == 0
when a periodicCallback is registered and shutdown hasn't started:
while (pendingSubprocessCount > 0 ||
(periodicCallback != NULL && !shutdownSequenceInProgress))
For every existing (periodicCallback == NULL) caller this reduces to
the exact original condition -- proven by the full local + pgaftest
regression run below, unchanged behavior. For a callback-driven
caller it also fixes a second, related latent case: pending
SubprocessCount legitimately dropping back to 0 without meaning
"permanently done" (e.g. the reconciler dropping its last membership,
expected to pick one back up later) no longer ends the loop either.
A second, smaller fix was needed alongside it: waitpid(-1, WNOHANG)
returns ECHILD immediately once truly no children exist at all --
the expected state before the first supervisor_add_service() call,
or between one service set fully exiting and the next being added --
which the existing code treated as fatal unless already shutting
down. Extended to treat "no children, still zero-registered,
periodicCallback in use" as the equivalent of "nothing to reap this
tick" (case 0): check signals, keep going, rather than erroring out.
Both new conditions were first written checking the raw
asked_to_stop/asked_to_stop_fast/asked_to_quit globals directly, which
looked right but caused a real, reproducible ~15-60s hang on shutdown
(caught by a very deliberate 5-minute local regression pass before
committing this, not by inspection): those globals are deliberately
self-clearing -- supervisor_handle_signals() resets whichever one
fired back to 0 immediately after processing it, precisely so a later,
second signal can be told apart from the first ("allow for processing
signals again", its own comment). Checking them directly in the loop
condition meant they read true for one iteration and false again on
the next, flipping the new disjunct back to "keep going" past the
point the loop should have exited, until the stuck-process escalation
timer eventually forced it dead via killpg() several seconds later.
Fixed by gating on supervisor->shutdownSequenceInProgress instead --
the field that actually stays true for the rest of the shutdown, the
same source supervisor_restart_service() already trusts for the
identical "are we shutting down" question a few functions above.
Verified: sh ci/banned.h.sh and citus_indent --check both clean; full
build against real PG17 in docker; pgaftest specs pass against PG17 --
basic_operation (28/28, exercises only the untouched static-array
path -- the regression-risk case for every non-archiver node kind),
archiver_wal_capture (3/3, including the SIGTERM-driven container
restart in test_002 -- consistent ~5s timing across repeated runs,
matching the pre-existing baseline exactly, not the ~15-60s hang the
first (buggy) version of this fix produced), archiver_basebackup_
generation (1/1), archiver_bootstrap_and_fast_forward (4/4), ssl_cert
(6/6, archiver + failover + cert-auth together).
Second review pass, focused on missed-reuse and duplication rather than correctness. Four concrete, well-scoped fixes: 1. file_utils.c gains write_file_atomic() -- a genuinely new, broadly useful facility this project didn't have: write-to-tmp-then-rename, the same atomicity write_file() alone doesn't provide. Extracted from four call sites that had each hand-rolled the identical fopen_with_umask/fformat.../fclose/rename sequence inline: service_archiver.c's service_archiver_persist_current_lsn() and service_archiver_maybe_persist_systemid(), and service_archiver_ reconciler.c's archiver_reconciler_write_tracking_file() and archiver_reconciler_write_routes_file(). The two simple, fixed- content writers in service_archiver.c now just sformat() into a small stack buffer and call write_file_atomic() directly; the two loop-over-memberships writers in service_archiver_reconciler.c build into a PQExpBuffer (matching pgctl.c's own existing use of the same type) and call it once with the accumulated content. 2. monitor.c's monitor_get_latest_basebackup_info() used its own ad hoc strdup()-into-context-then-manual-free() pattern in BasebackupInfoParseContext/parseBasebackupInfo, the only multi- column single-row parser in the file that does this -- every other one (parseNodeRegion, the FsmReachabilityParseContext family) has the parse callback write straight into the caller's own output buffers, since the caller already has them sized and ready. Not a live bug (every strdup path that succeeds also frees on the same function's return), but a real inconsistency with the file's own established idiom and a plausible spot for a future leak if an early return ever gets added between strdup and free. Now matches the rest of the file: storageLocation/source point directly at the caller's buffers, no heap allocation at all. 3. pg_walsender's cmd_base_backup.c partial_segment_real_length() hand-rolled fopen/malloc(CBB_WAL_SEGMENT_SIZE)/fread instead of using read_file() (file_utils.c), already linked into this binary and already used twice elsewhere in this same file. read_file() sizes its own allocation from the file's real size (fseek/ftell) rather than a hardcoded 16MB constant, and logs a real error on open failure instead of failing silently -- strictly better here, since the caller only ever calls this once bestPartial has already been confirmed to exist by the caller's own directory scan, so a fopen failure at this point is a genuine error worth logging, not an expected "doesn't exist yet" case. Investigated, not changed (lower value or not practically extractable given getopt_long's own single-table-per-parser shape): - cli_create_archiver (cli_create_node.c) hand-rolls the pathname- setup/pidfile-check/config-write bootstrap sequence inline instead of reusing cli_create_config/keeper_pg_init's pattern -- real, but a larger, higher-risk refactor of the top-level create-archiver flow, not a mechanical extraction; flagged as a real follow-up candidate. - cli_create_archiver_getopts's SSL-flag case block and cli_archiver_ serve_getopts's --pgdata/--version/--verbose/--quiet/--help block each duplicate scaffolding cli_common_keeper_getopts/cli_getopt_ pgdata already have, but getopt_long's own single-option-table-per- call design means extracting a shared piece requires restructuring those existing, widely-used functions to accept caller-supplied extra flags -- not attempted here given the blast radius. - pg_walsender's cmd_base_backup.c and wal_dir_scan.c both implement is_wal_segment_filename() and a similar directory-scan-for-newest- segment loop; the duplication is semantically justified (cmd_base_ backup.c's own version also needs to consider .partial segments, wal_dir_scan.c's doesn't) and the author already documents this tradeoff in a comment -- a shared, parameterized helper is possible but lower value given the two loops already diverge in a real way. Verified: sh ci/banned.h.sh and citus_indent --check both clean (one auto-fix applied via `make docker-indent` after the PQExpBuffer change); full build against real PG17 in docker; pgaftest specs pass against PG17: basic_operation (28/28, broad monitor.c coverage), archiver_wal_capture (3/3, exercises the reconciler's routes/tracking file writers directly), archiver_basebackup_generation (1/1, exercises monitor_get_latest_basebackup_info), archiver_bootstrap_and_fast_ forward (4/4, exercises the read_file()-based partial-segment path).
Follow-up to an earlier review that catalogued this project's own existing LANGUAGE sql functions (wal_archived, get_latest_basebackup, node_timeline_status's recursive CTE) as proof plpgsql is reserved for genuine procedural need -- RAISE EXCEPTION, an explicit lock gating a later statement, or looping over unknown cardinality -- everything else collapses into one query. That review flagged ~14 functions as convertible but never implemented any of them; this commit does, for the ones that convert with zero behavior change: - report_timeline_history, create_basebackup_policy, create_rclone_config: single INSERT [...] RETURNING, no branching. - archiver_remove_formation, set_archiver_policy, report_basebackup_synced, report_basebackup_remote_deleted, report_pitr_status, pitr_queue_command: single INSERT/UPDATE/DELETE (ON CONFLICT DO UPDATE or DO NOTHING where needed), no branching. - prune_archiver_wal: the IF oldest_startlsn IS NULL THEN RETURN 0 early exit was actually redundant -- "lsn < NULL" is NULL (falsy) for every row, so the plain DELETE already deletes nothing when no complete backup exists yet, same result without the branch. - basebackup_concurrency_available (added in an earlier commit this PR): converted via one CTE, same logic, no lock/branch needed. - pitr_next_command: the "peek old value, then clear" pattern doesn't need PL/pgSQL either -- a SELECT ... FOR UPDATE CTE captures the pre-update value (every CTE in one WITH clause sees the same query-start snapshot), a second UPDATE CTE conditionally clears it based on that captured value, and Postgres always executes a data-modifying CTE once included in the WITH clause regardless of whether the final SELECT reads its output -- documented guarantee, not an assumption. Deliberately NOT converted, despite being structurally simple INSERT/ UPDATE statements: set_basebackup_policy, set_rclone_config, archiver_add_storage, archiver_remove_storage, report_wal_received, report_basebackup_started, report_basebackup_completed, report_basebackup_deleted, create_archiver_node, remove_archiver_node, set_archiver_node_pitr_status, accept_timeline -- every one needs RAISE EXCEPTION on a not-found/invalid condition (or, for create_ archiver_node/report_basebackup_started, a row lock gating a later conditional raise), which plain SQL functions can't express at all. Also deliberately NOT converted: get_archiver_policy (and get_ basebackup_policy_for_group, which depends on it). Converting this one is not just a mechanical rewrite: its current per-row fallback (a group-level override row, once found, wins with ALL of its own columns as-is, even a NULL basebackuppolicyid it never explicitly set) differs in a real, user-visible way from a column-level COALESCE cascade (which would fall through to the formation-level or default basebackuppolicyid for that one column instead of returning NULL). That's a behavior change, not a refactor, and deserves its own dedicated test (a group override that sets archiverquorum without touching basebackuppolicyid) rather than landing silently inside a "pure reuse" pass -- left as plpgsql, flagged as a genuine follow-up candidate once that test exists. register_archiver was also left alone for the reason already noted before this PR started: its optional rclone_config-attach step reuses archiver_add_storage()'s own "config not found" validation, which a CTE-inlined equivalent would either duplicate or silently drop. Verified: sh ci/banned.h.sh and citus_indent --check both clean; full monitor extension build + regression suite (20/20) + isolation suite (6/6) against real PG17 via pg_virtualenv, including archiving_schema (exercises every converted function); full build against real PG17 in docker; pgaftest specs pass against PG17: archiver_wal_capture (3/3), archiver_basebackup_generation (1/1), archiver_basebackup_policy (2/2, exercises create_basebackup_policy directly), archiver_bootstrap_and_fast_forward (4/4), ssl_cert (6/6).
`create archiver --formation` previously required the target formation's
group to already exist, and there was no CLI-level way to attach an
archiver to a formation afterwards -- only raw SQL against the monitor.
Both gaps closed:
1. `pg_autoctl archiver formation add|remove|list`, replacing the earlier
flat `add-formation` prototype. `add`/`remove` reuse the existing
archiver_add_formation()/archiver_remove_formation() SQL functions via
new name-based overloads (an archiver's --name is all any CLI caller
ever has, never its internal archiverid); `list` needed no new backend
work at all -- pgautofailover.get_archivers()/monitor_get_archivers()
already existed, used only internally by `pg_autoctl watch` until now.
2. `create archiver`'s own per-formation attach step now retries (the
same ConnectionRetryPolicy machinery this CLI already uses elsewhere
for "monitor not reachable yet", discover_hostname) for up to 15
minutes instead of failing immediately when the target formation has
no group registered yet. Safe to retry unconditionally here: archiverId
was just resolved by this same command's own registration call, so any
failure from this point on can only mean "not ready yet", never a real
error. Removes the startup-ordering requirement entirely -- an archiver
container can now start before, after, or racing with its target
formation's own nodes.
3. `pg_autoctl archiver show basebackup|wal|state`, new:
- `basebackup`: thin wrapper over the already-existing
monitor_list_basebackups().
- `wal`: new pgautofailover.list_archiver_wal() SQL function (archiver_
wal had no read access at all outside internal report/prune
functions until now) + monitor_list_archiver_wal() C wrapper, grouped
one row per segment across every archiver holding it under quorum
rather than one row per (segment, archiver).
- `state`: this archiver's own identity and every formation/group
membership with FSM state, via the existing monitor_list_archiver_
memberships() (already used internally by the reconciler). The
rendering is one shared function, cli_print_archiver_state()
(cli_archiver.h), also called directly from `pg_autoctl show state`
(cli_show.c) when the local config's nodekind is "archiver" -- that
command's usual single-formation/group node table doesn't apply to
an archiver attached to more than one formation, so this is a real
behavioral aliasing, not just a doc note.
Docs: new pg_autoctl_archiver_formation.rst/pg_autoctl_archiver_show.rst
manual pages, pg_autoctl_archiver.rst toctree, refreshed the archiver
section of pg_autoctl.rst's pasted command-tree (that file has other,
pre-existing staleness elsewhere -- inspect/manual/node/basebackup-policy
-- out of scope here), and pgaftest.rst's own archiver-node section now
points at `archiver formation add` instead of raw SQL and describes the
retry instead of an ordering requirement.
Verified: sh ci/banned.h.sh and citus_indent --check both clean; full
native build of src/bin/pg_autoctl (-Wall -Werror) clean; every level of
the new command tree's --help output verified end-to-end against the
real native binary, matching the docs exactly. Could not run a real PG17
docker build or pgaftest specs this session -- Docker Hub was
unreachable (persistent TLS handshake timeout pulling debian:bookworm-
slim, 7 attempts over several hours) for the whole session from this
point on.
… reporting Replaces the archiver's direct-child pg_receivewal with a vendored, in-process pg_receivewal (vendor/pg_receivewal/) run by its own dedicated sibling controller process (service_archiver_pgreceivewal_ ctl.c), fixing a supervisor.c wildcard-waitpid reaping race and matching the project's existing service_postgres_ctl.c pattern. The vendored source adds a WalSegmentClosedHook (the missing upstream archive_command-equivalent) instead of any other modification, and carries small, clearly-marked cross-PG-version (14-19) compatibility shims. WAL-capture reporting is redesigned around a Unix-socket protocol (archiver_wal_notify.c) plus a new sibling WAL-cache scanner process (service_archiver_wal_scanner.c) that replaces the old inline, tick-coupled directory-scan fallback. Both pg_receivewal's live hook and the periodic scanner feed the same socket; the FSM tick drains it and reports the whole batch to the monitor in one round trip via a new report_wal_received_bulk() SQL function, instead of one round trip per segment. archiver_wal now also tracks each segment's system identifier (archiver_systemid.c), so WAL from a stale cluster incarnation can be told apart from the current one. Also: automatic archiver-bootstrap detection for `create postgres` (no more mandatory --from-archiver), backup-manifest support in pg_ walsender's BASE_BACKUP, streq()/explicit-column cleanups, and doc updates explaining the Postgres-protocol-only design and the pg_ receivewal vendoring rationale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… hang Two real bugs found and fixed while validating the WAL-reporting redesign, both root-caused against the actual failing behavior rather than guessed at: 1. cmd_base_backup.c sent the backup manifest as a second, separate CopyOutResponse/CopyDone phase after the tar stream's own CopyDone. Real Postgres (cross-checked against basebackup_copy.c in the local Postgres source tree) sends every archive AND the manifest within a single CopyOut stream: the manifest is announced with a CopyData['m'] (PqBackupMsg_Manifest) marker and its content chunks use the same 'd'-tagged framing as tar data, with exactly one CopyDone at the very end. The old, structurally wrong sequence caused a real pg_basebackup client to fail with an empty "backup failed:" error whenever a node bootstrapped from the archiver with a manifest requested. 2. pg_receivewal never resumed after an archiver process restart. service_archiver_start_pgreceivewal() (and the desired-state file it writes) is only ever invoked from an FSM transition *into* ARCHIVING_STATE -- never fired again on a restart where current_role is already "archiving" and the monitor keeps assigning the same state. Meanwhile service_archiver_loop()'s own shutdown path unconditionally stops pg_receivewal on every exit, so a restart with no transition left it stopped forever. Fixed by re-asserting the desired state once at capture startup whenever already assigned ARCHIVING_STATE (fsm_init_archiver() is idempotent, so this is a no-op on an ordinary cold start). Also: switched both the archiver's own pull from the primary and a node's bootstrap pull from the archiver to --wal-method=stream (now that pg_walsender serves real START_REPLICATION) instead of --wal- method=none, so a base backup's own start LSN never depends on a separate, independently-timed WAL capture having already covered it -- the direct cause of a "requested segment ... predates the oldest segment this archiver has captured" bootstrap hang. Added a primary/ replication-slot readiness preflight loop before forking pg_receivewal to close a related startup race. Kept the error-visibility logging added to cmd_base_backup.c's wire-sequence sends as a permanent improvement -- every send failure there used to be silent. Verified: archiver_bootstrap_and_fast_forward and archiver_wal_ capture's own restart-liveness test now pass; archiver_basebackup_ generation and basic_operation show no regression. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
"logs node2 contains" greps `docker compose logs node2`, which only
ever captures node2's own PID1 stdout (the "create = deferred"
entrypoint's own output) -- never a separate `exec`'d command's own
output, which goes to that exec's own pipe instead. test_003 was
exec'ing `pg_autoctl create postgres` and then checking for its own
"bootstrapping from it automatically" log line via "logs node2
contains" -- an assertion that could never pass regardless of whether
the automatic archiver-bootstrap-detection feature (keeper_should_
bootstrap_from_archiver, fsm_transition.c) actually worked, confirmed
by testing with a string that's always logged ("Using default") and
finding it missing too.
Fixed by redirecting create postgres's own stdout/stderr to a file and
grep-ing that file directly instead. Verified passing (isolated from
test_002's own separate, unrelated timing-margin flakiness) once fixed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… tune restart timing Real data-loss bug, found via direct SQL inspection of archiver_wal while chasing an intermittent archiver_wal_capture.pgaf timeout: the in-memory "high-water mark" (lastReportedWalFileName) that gated which drained WAL-notify messages made it into a tick's own batch assumed notifications arrive in non-decreasing filename order -- true for the old single-sorted-scan design, false once two independent producers (pg_receivewal's own live hook and service_archiver_wal_scanner.c's periodic backstop) feed the same best-effort socket. Sequence that lost a real segment: the live hook's own notification for segment N was silently dropped (nothing listening yet, right after a restart); the next segment's own notification succeeded and advanced the high-water mark past N; the scanner later correctly rediscovered N and tried to report it, but the stale high-water mark now silently excluded it from the batch, permanently. Fixed by removing the high-water-mark filter entirely -- every drained message is batched unconditionally now, correctness resting solely on the monitor-side INSERT's own ON CONFLICT DO NOTHING. Occasionally re-sending an already-known segment is cheap; silently losing archived WAL forever is not an acceptable trade for the marginal batch-size savings. Also, per discussion: - pgIsRunning is now reported as pg_receivewal's own real liveness (service_archiver_pgreceivewal_is_running()) instead of being hardcoded true, giving operators and tests a SQL-visible way to see it (pgautofailover.node.reportedpgisrunning) instead of only ever being inferable from log lines -- this is what surfaced the data- loss bug above in the first place. NodeIsHealthy()/NodeIsUnhealthy() (node_metadata.c) are updated to stop requiring pgIsRunning for a !hasPgData (archiver) row: what actually serves WAL to a FAST_FORWARD candidate is pg_walsender, a separate process pg_receivewal's own liveness has no bearing on, so tying FAST_FORWARD eligibility to it would incorrectly mark an archiver unhealthy exactly when pg_ receivewal is legitimately stopped (the group's primary just died) and the archiver is needed most. - wait_for_primary_and_slot_ready()'s own poll interval tightened from 1s to 250ms (service_archiver_pgreceivewal_ctl.c) -- same 20s overall bound, finer granularity, so the common restart case (slot already exists) doesn't pay up to a full extra second of pure waiting. - archiver_wal_capture.pgaf's own restart-liveness wait bumped from 30s to 60s to actually cover the documented worst case (up to 20s preflight bound plus the real connect-and-stream-and-report cycle that follows a restart), rather than guessing at a bigger number. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a second, lower-stakes message type to the WAL-notify socket protocol alongside SEGMENT: PROGRESS, carrying pg_receivewal's own raw stream position between segment boundaries. Sourced from a new hook point in the vendored pg_receivewal.c -- stop_streaming() already runs on every check-in, not just when a segment closes; the existing WalSegmentClosedHook only ever fired on the segment_finished branch, so the other branch had a hook added (WalProgressHook) rather than inventing a new call site. Throttled at the source (once every 5s, pgaf_hook_wal_progress()) since stop_streaming() can fire far more often than that under a busy primary. Deliberately NOT a replay/FAST_FORWARD/PITR target: pg_receivewal never parses WAL record content, so this LSN is not guaranteed to land on a genuine record boundary (the same reason service_archiver_ update_current_lsn() already refuses to use a still-open segment's own byte count as currentLSN). It lands in a new, clearly-separate column (archiver_node.lastprogresslsn/lastprogressat) via a new report_wal_ progress() SQL function, reported immediately as each PROGRESS message is drained (low-volume by the source-side throttle, no need to batch like SEGMENT messages) -- nothing in FAST_FORWARD eligibility or PITR targeting reads it, purely a lag/progress metric for operators and tests. Verified end-to-end against a live archiver: lastprogresslsn observed mid-segment, well short of the next segment boundary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cted output PG19's pg_lsn output type pads the low 32 bits to 8 hex digits (0/01000000) where PG14-18 don't (0/1000000) -- already handled elsewhere in this same expected file, but missed for the report_wal_ received_bulk() test rows added earlier this session when the pg17 result was copied over verbatim. Caught by a full PG19 image rebuild.
Pure whitespace/alignment fixes (continuation-line indentation, comment-block indentation) in the files touched by today's WAL- reporting and PROGRESS-message work -- no functional change.
Real, consistently-reproducing CI failure across every archiver job (PG14-19): pg_basebackup: error: could not get COPY data stream: / unexpected termination of replication stream: ERROR: requested WAL segment predates this archiver's captured history and will never become available. Root cause: yesterday's fix switched BOTH legs of archiver base-backup traffic to --wal-method=stream, but only one of them needed it. The archiver's own pull from the real primary (run_pg_basebackup(), service_archiver_basebackup.c) genuinely needed it -- that backup now embeds every byte of WAL from its own checkpoint through its own end LSN directly inside the backup's own pg_wal/ directory, making the stored backup fully self-contained. But a node bootstrapping FROM the archiver was also switched to --wal-method=stream, which asks pg_ walsender's own START_REPLICATION handler (cmd_start_replication.c) to background-stream that same range a second, redundant time -- and that handler only ever knows how to serve from the archiver's own WAL *cache* (pg_receivewal's own captured segments), which has no relationship to a specific backup's own already-embedded pg_wal/ range and, for a freshly-started archiver in particular, routinely doesn't extend back far enough. Reverted just that leg to --wal-method=none: the fetched backup is already replayable on its own, nothing further to stream concurrently, and this node's own ordinary catch-up streaming (against the real primary, once bootstrapped) covers whatever comes after the backup's own already-embedded end LSN, same as for any other standby. Verified locally: archiver_bootstrap_and_fast_forward and archiver_ wal_capture both pass.
pg_basebackup_fetch() had no preflight at all against a real, observed
startup race: a freshly-registered node's own pg_hba.conf entry on the
source can take a moment to propagate (HBA rules are written and the
config reloaded asynchronously). pg_receivewal already tolerates this
via its own internal reconnect loop (and, since yesterday's fix,
wait_for_primary_and_slot_ready()'s own preflight closes the common
case there too) -- but plain pg_basebackup has no retry of its own, so
a single race hit was fatal.
pgctl_identify_system() already exists for exactly this ("check that
HBA is ready" per its own comment) and is already used by several
other bootstrap paths in this codebase, just never by pg_basebackup_
fetch() itself -- the archiver's own pull from the real primary
(run_pg_basebackup(), service_archiver_basebackup.c) had no preflight
at all before this. Retrying it up to PG_BASEBACKUP_HBA_MAX_ATTEMPTS
times before launching pg_basebackup closes the gap for every caller
of pg_basebackup_fetch() at once. No extra sleep between attempts:
each call's own connection already carries pgsql_init()'s "interactive"
retry policy (~2s of internal backoff), so an added outer sleep would
only compound that delay rather than add useful coverage.
Verified locally: archiver_bootstrap_and_fast_forward, archiver_
basebackup_generation, and basic_operation all pass.
The manifest-in-same-stream fix applied the PG15+ single-combined-
CopyOut design unconditionally, folding manifest bytes into the tar's
own untagged CopyOut for pre-PG15 clients too. PG14 pg_basebackup
reads that stream as tar-only and chokes on the manifest bytes
("invalid tar block header size"). Split the manifest handling on
CBB_USE_ARCHIVE_FRAMING: PG15+ keeps the single combined stream,
pre-PG15 goes back to a second, independent CopyOutResponse/CopyDone
pair for the manifest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Archiving & Disaster Recovery, Milestones 1–5 of the design (schema through base-backup generation + policy), plus the docs coverage for all of it.
An archiver is a new node kind that captures a group's WAL continuously (
pg_receivewal, via the same replication-slot mechanism a standby already uses) and produces periodic base backups (live or replayed locally from its own WAL cache), independent of whether any standby is healthy or even present. A new standalone binary,pg_walsender, serves that captured data back out over a real (subset of the) PostgreSQL replication protocol, so a realpg_basebackup, a real streaming standby, or this project's ownrestore_commandcan all talk to an archiver with no archiver-aware code beyond finding it.This is not ready to merge — opening now for early review of direction and approach while the remaining work continues. See "Known gaps" below.
What's in this PR
pgautofailoverSQL schema for archivers —archiver,basebackup_policy,archiver_policy,archiver_wal,basebackup,wal_archived(), retention/pruning functions.service_archiver,archiver serve,pg_walsender(M2): the keeper-sideARCHIVINGFSM state;service_archiver.c's WAL-capture loop; a brand-new standalone binary (src/bin/pg_walsender/) implementing enough of the replication wire protocol from scratch (no frontend-linkable server-side implementation exists anywhere in PostgreSQL to link against) —IDENTIFY_SYSTEM,SHOW,BASE_BACKUP,TIMELINE_HISTORY,CREATE/READ_REPLICATION_SLOT,START_REPLICATION, and aFETCH_FILEside channel forrestore_command.pg_autoctl node runsupport forkind = archiver(M3).archiver_wal/wal_archived()tracking, re-pointingpg_receivewalat a new primary after a failover.live(realpg_basebackupagainst a healthy node) andreplay/volatile(extract the last backup, replay locally captured WAL forward against a throwaway staging instance, snapshot over loopback, discard) sources; policy-driven scheduling (frequency,onpromotion) and retention (maxcount,maxage); new CLI (pg_autoctl create/show/set basebackup-policy,--basebackup-policyoncreate archiver).pg_autoctl create postgres --from-archiver(bootstrap a new node straight from an archiver's cache) andFAST_FORWARDreusing an archiver as a WAL source during a multi-standby election, both via the same well-known-port resolution trick, no archiver-specific code in the ordinary standby-init/fast-forward paths themselves.SINGLEandPRIMARYforever (group_state_machine.c), and the replay staging instance failed to start under SSL (missing certs it has no reason to have).archiving-internals.rst) written as the technical reference/extension point for the milestones after this one; a new Operations page;ARCHIVINGstate coverage in the FSM docs (plus a full regeneration of the five FSM mermaid diagrams frompg_autoctl inspect fsm mermaid— real drift was found and fixed there, unrelated to archiving); fault-tolerance coverage; a rewritten intro with new architecture diagrams; and a small site-wide docs feature (click-to-zoom on figures, generalizing the zoom Mermaid diagrams already had).Known gaps — why this isn't ready yet
pg_walsender(all-new wire-protocol code) and the FSM fix ingroup_state_machine.c.conf.pychange (adds the click-to-zoom JS/CSS): Sphinx's incremental build doesn't reliably re-emit the<script>/<link>tags on every already-built page just becauseconf.pychanged — only pages whose own.rstsource changed get regenerated. If a localdocs/_buildpredates this PR,make -C docs htmlalone won't retrofit the zoom feature onto older pages; runmake -C docs clean html(or deletedocs/_build) once to pick it up everywhere.archiving-internals.rst's "Extension points" section is written to be where that work plugs in.Testing
New/updated
pgaftestspecs:archiver_wal_capture,archiver_bootstrap_and_fast_forward,archiver_basebackup_generation,archiver_basebackup_policy— all passing against a from-scratch--no-cacheDocker rebuild.citus_indent --checkclean.sphinx-build -W --keep-goingclean (no warnings, no broken references).