From de8268b69969531952ca411b5c1490b4bb660b7f Mon Sep 17 00:00:00 2001 From: Jesper St John Date: Mon, 19 Oct 2026 15:35:59 +0200 Subject: [PATCH v1] pg_basebackup: Create the WAL streaming slot before starting the backup With --wal-method=stream, pg_basebackup created its replication slot only after BASE_BACKUP had returned the backup's start position. A checkpoint running in between could remove WAL from that position onwards, and the backup then failed with "requested WAL segment ... has already been removed". Open the WAL streaming connection and create its slot before sending BASE_BACKUP, rather than after it has returned the start position, so the slot reserves all the WAL the backup needs. Add a TAP test that uses an injection point to hold slot creation while WAL is advanced and a checkpoint is run. --- src/bin/pg_basebackup/Makefile | 4 +- src/bin/pg_basebackup/meson.build | 2 + src/bin/pg_basebackup/pg_basebackup.c | 96 ++++++++++++------- .../pg_basebackup/t/012_wal_slot_timing.pl | 82 ++++++++++++++++ 4 files changed, 149 insertions(+), 35 deletions(-) create mode 100644 src/bin/pg_basebackup/t/012_wal_slot_timing.pl diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile index df94fc27d02..12ff32e329b 100644 --- a/src/bin/pg_basebackup/Makefile +++ b/src/bin/pg_basebackup/Makefile @@ -12,13 +12,15 @@ PGFILEDESC = "pg_basebackup/pg_receivewal/pg_recvlogical - streaming WAL and backup receivers" PGAPPICON=win32 -EXTRA_INSTALL=contrib/test_decoding +EXTRA_INSTALL=contrib/test_decoding \ + src/test/modules/injection_points subdir = src/bin/pg_basebackup top_builddir = ../../.. include $(top_builddir)/src/Makefile.global # make these available to TAP test scripts +export enable_injection_points export LZ4 export TAR # Note that GZIP cannot be used directly as this environment variable is diff --git a/src/bin/pg_basebackup/meson.build b/src/bin/pg_basebackup/meson.build index d70ce5786a2..c10a56512a0 100644 --- a/src/bin/pg_basebackup/meson.build +++ b/src/bin/pg_basebackup/meson.build @@ -96,10 +96,12 @@ tests += { 'env': {'GZIP_PROGRAM': gzip.found() ? gzip.full_path() : '', 'TAR': tar.found() ? tar.full_path() : '', 'LZ4': program_lz4.found() ? program_lz4.full_path() : '', + 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no', }, 'tests': [ 't/010_pg_basebackup.pl', 't/011_in_place_tablespace.pl', + 't/012_wal_slot_timing.pl', 't/020_pg_receivewal.pl', 't/030_pg_recvlogical.pl', 't/040_pg_createsubscriber.pl', diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index c3b87a19e76..e48e75ddd6d 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -165,6 +165,9 @@ static bool found_existing_xlogdir = false; static bool made_tablespace_dirs = false; static bool found_tablespace_dirs = false; +/* Connection for the background WAL receiver, see PrepareLogStreamer() */ +static PGconn *bgconn = NULL; + /* Progress indicators */ static uint64 totalsize_kb; static uint64 totaldone; @@ -607,6 +610,55 @@ LogStreamerMain(logstreamer_param *param) return 0; } +/* + * Open the connection for the background WAL receiver and create its + * replication slot, if one is needed. + * + * This must be done before BASE_BACKUP is sent: the slot reserves WAL from + * its creation onwards, so creating it first guarantees that no WAL from the + * backup's start position onwards can be removed before the receiver starts + * streaming. If the slot were created only after BASE_BACKUP had returned + * the start position, a checkpoint in between could remove WAL the backup + * needs. + */ +static void +PrepareLogStreamer(void) +{ + /* Get a second connection */ + bgconn = GetConnection(); + if (!bgconn) + /* Error message already written in GetConnection() */ + exit(1); + + /* Temporary replication slots are only supported in 10 and newer */ + if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS) + temp_replication_slot = false; + + /* + * Create replication slot if requested + */ + if (temp_replication_slot && !replication_slot) + replication_slot = psprintf("pg_basebackup_%u", + (unsigned int) PQbackendPID(bgconn)); + if (temp_replication_slot || create_slot) + { + if (!CreateReplicationSlot(bgconn, replication_slot, NULL, + temp_replication_slot, true, true, false, + false, false)) + exit(1); + + if (verbose) + { + if (temp_replication_slot) + pg_log_info("created temporary replication slot \"%s\"", + replication_slot); + else + pg_log_info("created replication slot \"%s\"", + replication_slot); + } + } +} + /* * Initiate background process for receiving xlog during the backup. * The background stream will use its own database connection so we can @@ -620,7 +672,10 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier, logstreamer_param *param; char statusdir[MAXPGPATH]; + Assert(bgconn != NULL); + param = pg_malloc0_object(logstreamer_param); + param->bgconn = bgconn; param->timeline = timeline; param->sysidentifier = sysidentifier; param->wal_compress_algorithm = wal_compress_algorithm; @@ -639,46 +694,12 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier, pg_fatal("could not create pipe for background process: %m"); #endif - /* Get a second connection */ - param->bgconn = GetConnection(); - if (!param->bgconn) - /* Error message already written in GetConnection() */ - exit(1); - /* In post-10 cluster, pg_xlog has been renamed to pg_wal */ snprintf(param->xlog, sizeof(param->xlog), "%s/%s", basedir, PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); - /* Temporary replication slots are only supported in 10 and newer */ - if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS) - temp_replication_slot = false; - - /* - * Create replication slot if requested - */ - if (temp_replication_slot && !replication_slot) - replication_slot = psprintf("pg_basebackup_%u", - (unsigned int) PQbackendPID(param->bgconn)); - if (temp_replication_slot || create_slot) - { - if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, - temp_replication_slot, true, true, false, - false, false)) - exit(1); - - if (verbose) - { - if (temp_replication_slot) - pg_log_info("created temporary replication slot \"%s\"", - replication_slot); - else - pg_log_info("created replication slot \"%s\"", - replication_slot); - } - } - if (format == 'p') { /* @@ -1970,6 +1991,13 @@ BaseBackup(char *compression_algorithm, char *compression_detail, compression_detail); } + /* + * If streaming WAL, create the replication slot before starting the + * backup, so that it protects all the WAL the backup needs. + */ + if (includewal == STREAM_WAL) + PrepareLogStreamer(); + if (verbose) pg_log_info("initiating base backup, waiting for checkpoint to complete"); diff --git a/src/bin/pg_basebackup/t/012_wal_slot_timing.pl b/src/bin/pg_basebackup/t/012_wal_slot_timing.pl new file mode 100644 index 00000000000..0de5e24058b --- /dev/null +++ b/src/bin/pg_basebackup/t/012_wal_slot_timing.pl @@ -0,0 +1,82 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Check that pg_basebackup's replication slot reserves WAL from before the +# backup's start position. If the slot were created only after BASE_BACKUP +# had returned the start position, a checkpoint running in between could +# remove the WAL the backup needs, and streaming it would fail. +# +# We hold the slot creation with an injection point, force WAL past several +# segments and checkpoint, then let the slot creation continue. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $tempdir = PostgreSQL::Test::Utils::tempdir; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init(allows_streaming => 1); +$node->append_conf( + 'postgresql.conf', q{ +wal_keep_size = 0 +checkpoint_timeout = 1h +}); +$node->start; + +if (!$node->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$node->safe_psql('postgres', 'CREATE EXTENSION injection_points'); +$node->safe_psql('postgres', + "SELECT injection_points_attach('replication-slot-create-begin', 'wait')" +); + +my $backupdir = "$tempdir/backup"; +my ($stdout, $stderr); +my $h = IPC::Run::start( + [ + 'pg_basebackup', '--no-sync', + '--checkpoint' => 'fast', + '--pgdata' => $backupdir, + '--wal-method' => 'stream', + '--dbname' => $node->connstr('postgres'), + ], + '>' => \$stdout, + '2>' => \$stderr); + +# Wait until the walsender is about to create pg_basebackup's slot. +$node->wait_for_event('walsender', 'replication-slot-create-begin'); + +# Move WAL forward by several segments and checkpoint, so that every segment +# not protected by a slot or wal_keep_size is removed or recycled. +$node->advance_wal(3); +$node->safe_psql('postgres', 'CHECKPOINT'); + +$node->safe_psql('postgres', + "SELECT injection_points_detach('replication-slot-create-begin')"); +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('replication-slot-create-begin')"); + +my $ok = $h->finish; +ok($ok, 'pg_basebackup succeeds with WAL activity during slot creation') + or diag("pg_basebackup stderr:\n$stderr"); +unlike( + $stderr, + qr/has already been removed/, + 'no WAL needed by the backup was removed'); + +# pg_verifybackup also parses the WAL the backup needs, so this fails if +# any of it is missing. +$node->command_ok([ 'pg_verifybackup', $backupdir ], + 'backup with streamed WAL verifies'); + +done_testing(); -- 2.39.5