Hello Andrey,
Thanks for your comments and for your patience -- I learn Postgres code
as I go.
On 10/09/2026 09:01, Andrey Borodin wrote:
I think the client-side fix is worth backpatching, even if this broader
work stays on HEAD.
I agree, and I'm attaching a separate patch for that. It's only been
tested against HEAD at the moment, but if it looks acceptable, I'll go
ahead and test it all the way back to v15. It also makes the current
documentation correct in that the slot is created _before_ the backup,
and does appear to be an easier way to resolve the race condition; both
of these points you raised in [1].
One build note: my build warned about the missing declaration of
BackupCtlShmemCallbacks. xlogbackup.c needs storage/subsystems.h for it.
I followed the suit of slot.c where ReplicationSlotsShmemCallbacks is
also undeclared, generates a similar warning, and seems to be acceptable
(for whatever reason). I'm happy to deviate from the pattern I am
reusing if that's the advice.
I'm not addressing your other comments here since they may become moot
if the client-side patch is accepted.
Cheers
Nick
[1]
https://www.postgresql.org/message-id/[email protected]
From cc87fe5e053c063c98b95abc3d76c432e848d649 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <[email protected]>
Date: Fri, 11 Sep 2026 15:18:03 +0100
Subject: [PATCH v1] pg_basebackup to create replication slot early
Try to create the replication slot, if requested, before requesting a
checkpoint. This addresses the WAL recycle race when multiple basebackups are
executed concurrently.
Suggested-by: Andrey Borodin <[email protected]>
Backpatch-through: 15
---
src/bin/pg_basebackup/pg_basebackup.c | 81 +++++++++++++++------------
1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c
b/src/bin/pg_basebackup/pg_basebackup.c
index c3b87a19e76..35d89695509 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -613,7 +613,8 @@ LogStreamerMain(logstreamer_param *param)
* stream the logfile in parallel with the backups.
*/
static void
-StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
+StartLogStreamer(PGconn *walconn, char *startpos, uint32 timeline,
+ char *sysidentifier,
pg_compress_algorithm wal_compress_algorithm,
int wal_compress_level)
{
@@ -625,6 +626,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char
*sysidentifier,
param->sysidentifier = sysidentifier;
param->wal_compress_algorithm = wal_compress_algorithm;
param->wal_compress_level = wal_compress_level;
+ param->bgconn = walconn;
/* Convert the starting position */
if (!pg_parse_lsn(startpos, ¶m->startptr))
@@ -639,46 +641,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')
{
/*
@@ -1754,6 +1722,7 @@ BaseBackup(char *compression_algorithm, char
*compression_detail,
int writing_to_stdout;
bool use_new_option_syntax = false;
PQExpBufferData buf;
+ PGconn *walconn = NULL;
Assert(conn != NULL);
initPQExpBuffer(&buf);
@@ -1970,6 +1939,46 @@ BaseBackup(char *compression_algorithm, char
*compression_detail,
compression_detail);
}
+ /* If we were asked to stream WAL, create a separate connection for
that */
+ if (includewal == STREAM_WAL)
+ {
+ walconn = GetConnection();
+ if (!walconn)
+ /* Error message already written in GetConnection() */
+ exit(1);
+
+ /*
+ * If we need to create a slot, do it now, before requesting a
checkpoint,
+ * to ensure the WAL we want is not removed until we actually
start
+ * streaming.
+ */
+
+ /* Temporary replication slots are only supported in 10 and
newer */
+ if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS)
+ temp_replication_slot = false;
+
+ if (temp_replication_slot && !replication_slot)
+ replication_slot = psprintf("pg_basebackup_%u",
+
(unsigned int) PQbackendPID(walconn));
+ if (temp_replication_slot || create_slot)
+ {
+ if (!CreateReplicationSlot(walconn, 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 (verbose)
pg_log_info("initiating base backup, waiting for checkpoint to
complete");
@@ -2100,7 +2109,7 @@ BaseBackup(char *compression_algorithm, char
*compression_detail,
wal_compress_level = 0;
}
- StartLogStreamer(xlogstart, starttli, sysidentifier,
+ StartLogStreamer(walconn, xlogstart, starttli, sysidentifier,
wal_compress_algorithm,
wal_compress_level);
}
--
2.50.1 (Apple Git-155)
