Skip to content

Add PostgreSQL 17, 18, and 19 support + graceful keeper shutdown#1144

Open
dimitri wants to merge 13 commits into
mainfrom
fix/pg17-18-19-support
Open

Add PostgreSQL 17, 18, and 19 support + graceful keeper shutdown#1144
dimitri wants to merge 13 commits into
mainfrom
fix/pg17-18-19-support

Conversation

@dimitri

@dimitri dimitri commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Add Postgres 17, 18, and 19 support

This PR extends pg_auto_failover to compile and run correctly on PostgreSQL 17, 18, and 19, and updates CI to cover all three new versions.

Changes

Core compatibility

  • version_compat.h: raise upper bound to PG20 so PG19 compiles
  • Makefile: add PG17–19 to PGVERSIONS; use CITUSTAG=v14.1.0 for PG18/19 (Citus v13 does not support PG18+)

PG17 fix — dbname in primary_conninfo (fixes #1116)

PG17 introduced synchronized_standby_slots for logical replication slot failover. When primary_conninfo lacks a dbname keyword the slot sync worker silently skips slot synchronisation. The fix threads dbname through ReplicationSource so it is always included in the generated primary_conninfo.

PG18 fix — received_tli NULL during WALRCV_CONNECTING

PG18 added a new WALRCV_CONNECTING state to pg_stat_wal_receiver where received_tli is NULL. Querying it unconditionally caused a crash. The fix guards against NULL and falls back to pg_control_checkpoint().timeline_id during that transient state.

PG19 fixes — failover stall and PgStartTime shim

  • received_tli > 0 filter: PG19 changed how WAL receiver state is reported; guard with received_tli > 0 to avoid acting on a stale receiver row, which caused the FSM to stall during failover.
  • Remove the PgStartTime → MyStartTimestamp shim: PG19 dropped PgStartTime; the shim in version_compat.h caused a double-definition error.
  • Fix pidfile race in service restart: atomic write + defensive retry, surfaced by PG19's faster startup.
  • Fix zero-warning policy violations: old-style function definitions and vendored fallthrough macro, flagged by PG19's stricter compiler defaults.

CI updates

  • run-tests.yml: add PG17 to all test matrices; add PG18/19 build+test jobs with correct CITUSTAG per version.
  • run-pgaftest.yml: extend pgaftest matrix to PG18/19; bump the pgaftest builder base image to PG18.
  • PG19+Citus jobs run with continue-on-error: true — Citus does not yet fully support PG19.
  • Remove PG19+Citus from the main job matrix once it was confirmed that Citus HEAD does not build on PG19 yet.

Diagnostic spec (excluded from CI)

tests/tap/specs/debug_failover_pg19.pgaf — a focused two-node failover spec used to reproduce and verify the PG19 stall fix locally. It is not included in any CI schedule.

Testing

All existing test suites pass on PG14–17. PG18 passes the full test suite including Citus. PG19 passes all non-Citus tests; Citus jobs are gated with continue-on-error pending upstream Citus PG19 support.

@dimitri dimitri self-assigned this Jul 12, 2026
@dimitri dimitri added the enhancement New feature or request label Jul 12, 2026
@dimitri dimitri changed the title Add PostgreSQL 17, 18, and 19 support Add PostgreSQL 17, 18, and 19 support + graceful keeper shutdown Jul 12, 2026
@dimitri dimitri force-pushed the fix/pg17-18-19-support branch from 8f580ff to 56dbc0a Compare July 13, 2026 02:56
dimitri added 10 commits July 13, 2026 04:57
@dimitri
Add Postgres 17, 18, 19 support
cb82121 
- Raise version_compat.h upper bound to PG20 (allows PG19 compilation)
- Add PG19 to PGVERSIONS in Makefile; use Citus v14.1.0 for PG18/19
- Extend CI matrices (run-tests.yml, run-pgaftest.yml) to cover PG18/19
  with correct Citus version per release; bump pgaftest builder to PG18
- Fix #1116: thread dbname through ReplicationSource so primary_conninfo
  includes a dbname keyword, required by PG17 synchronized_standby_slots
  for logical replication slot failover
- Guard pg_stat_wal_receiver.received_tli against NULL: PG18 introduced
  WALRCV_CONNECTING state where received_tli is NULL; fall back to
  pg_control_checkpoint() timeline_id during that transient state

Fixes #1116
@dimitri
Fix compilation and test failures for Postgres 19
b128fab 
PG19 requires several API and build system adaptations:

Build system:
- Remove explicit -std=c99 from Makefile.common and monitor/Makefile;
  pg_config --cflags now provides the right standard per version
  (gnu23 for PG19, c99/c11 for older versions)
- Add libnuma-dev to Dockerfile build deps (PG18+ links against libnuma)
- Support CITUSTAG=none in Dockerfile to skip Citus build for PG versions
  not yet supported by any Citus release (PG19)
- Update CI case statements: 18 -> v14.1.0, 19 -> none

Signal handlers (PG19 changed SIGNAL_ARGS to two-argument SA_SIGINFO form):
- Use SIGNAL_ARGS macro in all signal handler signatures
  (signals.c catch_reload/int/term/quit/quit_and_exit, watch.c catch_sigwinch)
- Use PG_SIG_IGN instead of SIG_IGN with pqsignal() in health_check_worker.c
- Add backward-compat PG_SIG_IGN/PG_SIG_DFL macros to version_compat.h
  for builds against PG < 19

Monitor extension (PG19 API changes):
- LWLockNewTrancheId() now takes the tranche name; LWLockRegisterTranche()
  is removed — add LWLockNewTrancheIdCompat() wrapper in version_compat.h
- ShmemInitHash() dropped the separate init/max size params — add
  ShmemInitHashCompat() wrapper
- WAIT_EVENT_CLIENT_READ moved to utils/wait_event.h; include explicitly
- storage/lock.h and storage/lmgr.h no longer pulled in transitively;
  add explicit includes to metadata.c and health_check_worker.c
- PgStartTime renamed to MyStartTimestamp — compat define in version_compat.h
- utils/timestamp.h no longer included transitively — add in version_compat.h
- pg_fallthrough macro used instead of /* FALLTHROUGH */ comment;
  add backward-compat pg_fallthrough define in version_compat.h for PG < 12
- Fix checkPgAutoFailoverVersion() old-style function definition

Regression tests:
- PG19 changed pg_lsn display format (e.g. 0/0 -> 0/00000000);
  add expected/pg19/expected/ with adjusted output for monitor and
  fast_forward tests; Makefile auto-selects this dir for PG >= 19

All 9 installcheck tests pass on PG16, PG17, PG18, and PG19.
@dimitri
Fix zero-warning policy: old-style function defs and vendored fallthr…
9c3b7b2 
…ough
@dimitri
Fix pidfile race in service restart: atomic write + defensive retry
f3f3946 
supervisor_update_pidfile() was rewriting the pidfile using O_TRUNC,
which zeros the file before writing the new content.  If cli_do_service
restart read the pidfile in that brief window it saw an empty file and
called log_fatal immediately.

Primary fix: write to supervisor.pidfile.tmp then rename(2) into place.
POSIX rename is atomic so readers always see a complete file.

Defensive fix: retry supervisor_find_service_pid up to 10 times with
10ms sleeps in cli_do_service_restart before treating the lookup as a
fatal error.
@dimitri
Fix PG19 failover stall: use received_tli > 0 to filter wal receiver …
d689018 
…state

The previous fix for PG19's WALRCV_CONNECTING state used:

  WHERE status <> 'connecting'

to exclude the connecting row and fall back to pg_control_checkpoint().
This is wrong when the WAL receiver restarts after having already
streamed: PG19 retains receivedTLI in shared memory across WAL receiver
restarts (WalRcvDie() does not reset it), so a restarting receiver in
WALRCV_CONNECTING state still has received_tli=3 from the previous
session, but the WHERE filter drops that row and the fallback returns
timeline_id=2 from the last restartpoint.

The monitor then sees node2 reporting TLI=2 while node1's last report
was TLI=3, which prevents the automatic failover from completing within
the 90-second timeout.

Fix: filter on received_tli > 0 instead of status <> 'connecting'.
This correctly handles both cases:
- First-ever WAL receiver (shared memory zeroed at startup):
  received_tli=0 → filtered → falls back to pg_control_checkpoint()
- WAL receiver restart after streaming TL3:
  received_tli=3 retained from previous session → not filtered → returns 3
@dimitri
Fix PG19 failover stall: remove wrong PgStartTime→MyStartTimestamp shim
7113047 
PgStartTime (the postmaster startup time) still exists in PG19 via
utils/timestamp.h.  The earlier shim incorrectly aliased it to
MyStartTimestamp, which is the *per-backend* start time set by
InitProcessGlobals() in every new process.

With the shim in place, NodeIsUnhealthy() compared health-check
timestamps against 'when did this backend connection start?'  A backend
forked to handle a node_active call is always younger than the monitored
healthCheckTime, so the condition

    TimestampDifferenceExceeds(PgStartTime, node->healthCheckTime, 0)

was permanently false in PG19 — the monitor marked an unhealthy primary
but the FSM never advanced to prepare_promotion because NodeIsUnhealthy
kept returning false.  The startup-grace-period check was also broken by
the same substitution.

Fix: delete the shim.  PgStartTime remains valid in PG19 and refers to
the same postmaster start time as in earlier releases.
@dimitri
Add debug_failover_pg19 diagnostic spec (excluded from CI schedule)
f45be9b 
Documents the investigation technique used to root-cause the PG19
failover stall: polling pg_stat_wal_receiver, pg_stat_activity, and
pgautofailover.node every few seconds to observe monitor state in real
time.  Useful as a reusable template for debugging future FSM stalls.

Not listed in tests/tap/schedule; run on demand:
  pgaftest run tests/tap/specs/debug_failover_pg19.pgaf
@dimitri
CI: use CITUSTAG=main for PG19, add citus jobs with continue-on-error
e94cb75 
PG19 images were built with CITUSTAG=none, so the citus test job ran
against an image with no Citus installed and immediately failed.

Changes:
- Dockerfile: when CITUSTAG=main, pass --without-pg-version-check to
  ./configure so Citus main builds against PG19 which has no stable
  Citus release yet.
- Makefile BUILD_ARGS_pg19: none → main.
- run-tests.yml: continue-on-error for the PG19 × citus job; Citus
  main may not always be stable on the latest Postgres beta.
- run-pgaftest.yml: same CITUSTAG switch in all three build case
  statements (run, debian, testrun targets); add PG19 citus-1 and
  citus-2 matrix entries with allow_failure: true so they show up in
  CI without blocking the overall build.
@dimitri
CI: revert PG19 CITUSTAG to none — Citus main does not build on PG19 yet
69a5556 
Citus main has compile errors against PG19 (PageSetChecksumInplace
removed, pgstat_report_vacuum signature changed, get_relation_info_hook
type gone, tuplesort/tuplestore API changes).  --without-pg-version-check
only bypasses the version guard; it cannot fix compilation.

Keep CITUSTAG=none for PG19 in the Makefile and workflow case statements.
The Dockerfile --without-pg-version-check branch and the continue-on-error
matrix entries for PG19 citus jobs are retained so the wiring is ready
when Citus main eventually supports PG19.
@dimitri
CI: remove PG19+citus from job matrices
3a367c7 
Citus does not yet support Postgres 19 — no release and the main branch
does not compile against it either.  The PG19 citus-1/citus-2 matrix
entries (with allow_failure/continue-on-error) were left behind when
CITUSTAG was reverted to none.  Remove them so those jobs no longer
appear in CI at all.
@dimitri dimitri force-pushed the fix/pg17-18-19-support branch from 56dbc0a to 3a367c7 Compare July 13, 2026 02:58
dimitri added 3 commits July 13, 2026 15:53
@dimitri
tests: move Citus DROP TABLE out of teardown into an explicit test step
1feedb4 
teardown_module() is not a safe place to run DDL that depends on the
cluster being in a clean state.  If the final test perturbs the cluster
(e.g. test_011_start_worker2b_again reattaches a failed node), Citus
metadata sync may still be in progress when teardown fires immediately
afterwards.  The DROP TABLE then fails with:

  ObjectNotInPrerequisiteState: <node> is a metadata node, but is out of sync

Because the exception is raised before cluster.destroy(), all background
pg_autoctl and postgres processes stay alive inside the test container,
keeping the docker run from exiting.  The job then burns its remaining
CI budget until the 15-minute hard timeout fires — consistently across
PG14, 15, 16, and 17.

Fix: move wait_until_metadata_sync() + DROP TABLE into a numbered test
step that runs while the cluster is in a known-stable state, and reduce
teardown_module() to cluster.destroy() only.

- test_basic_citus_operation: test_015_drop_table (after coordinator failover)
- test_citus_multi_standbys:  test_012_drop_table (after worker2b rejoins as secondary)
- test_citus_force_failover:  test_004b_drop_table (before node-drop tests)
- test_nonha_citus_operation: test_007b_drop_table (before primary failures)
@dimitri
tests: fix five categories of spurious CI failures
4f2073e 
1. basic_operation.pgaf test_022_detect_network_partition: sleep 3s → 30s

   The monitor assigns demote_timeout (and node3 reaches wait_primary) on
   the monitor/node3 side.  Node2's local keeper detects connection loss
   independently via network_partition_timeout (default 20s, defaults.h).
   Those two clocks are decoupled: 3s was not enough for node2's keeper to
   fire its own partition check and call fsm_stop_postgres.  30s covers
   the worst case (20s timeout + keeper loop interval + Postgres shutdown).

2. wait_until_assigned_state: add or_state= keyword argument

   The FSM can transition draining → demote_timeout in the same wall-clock
   second (monitor event log confirms same timestamp).  A 100ms poll loop
   can miss draining entirely.  or_state= lets callers accept either state.
   Used in test_003_002_stop_primary.

3. has_needed_replication_slots: 5s retry on OperationalError

   A newly-joined async or sync standby node can transiently refuse
   connections right after its FSM state converges to secondary.
   wait_until_state checks pg_auto_failover state, not psycopg2
   connectivity.  Retry the pgmajor() call for up to 5s (with cache
   invalidation) before surfacing the exception.

4. DataNode.run_sql_query_retry: new helper for transient connectivity

   Like run_sql_query but retries on OperationalError and
   CannotConnectNow (ERRCODE 57P03, 'database system is starting up')
   for up to a caller-specified timeout with 0.5s back-off.

5. test_nonha_citus_operation test_004: use run_sql_query_retry (30s)

   create_distributed_table internally connects to each worker node.
   Even after worker pg_autoctl state reaches 'single', Postgres can
   still report 'system is starting up' at the Citus inter-node level.
   Retry for 30s instead of failing immediately.
@dimitri
tests: fix Citus DROP TABLE race against metadata sync
ff95fbe 
wait_until_metadata_sync() reflects the coordinator's view of sync
completion, but a recently-rejoined worker (e.g. after re-attach as
secondary) may still be applying Citus metadata updates when the
coordinator considers sync done.  DROP TABLE then fails with:

  ObjectNotInPrerequisiteState: <node> is a metadata node, but is out of sync

Two related fixes:

1. Add DataNode.citus_run_ddl_after_sync(query, timeout=60): a helper
   that calls wait_until_metadata_sync() then the DDL, and retries the
   whole pair with 1s back-off if ObjectNotInPrerequisiteState is raised,
   for up to timeout seconds.  Use it everywhere DROP TABLE t1 appears
   after a metadata-sync wait.

2. Remove test_004b_drop_table from test_citus_force_failover: this test
   intentionally leaves workers in disrupted states (worker1a dead, worker1b
   in wait_primary with no secondary).  There is no clean stable point to
   run Citus DDL here; DROP TABLE coverage is provided by the other citus
   test files.

Verified: 77 tests pass cleanly across two consecutive local runs on PG16.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dimitri dimitri

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dimitri