From: Nick Ivanov <nick.ivanov@enterprisedb.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Álvaro Herrera <alvherre@kurilemu.de>
Cc: pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>
Subject: Re: Possible race condition in pg_basebackup
Date: Fri, 11 Sep 2026 17:25:06 +0100
Message-ID: <985de9f0-cbb6-4235-a6cd-32242f74e1f3@enterprisedb.com> (raw)
In-Reply-To: <801646A4-65A3-4DF1-84CD-7BF836E178BD@yandex-team.ru>
References: <aoh9KdCo12Y07mZO@alvherre.pgsql>
<5516902D-65A5-4C61-8568-E32F111C89EF@yandex-team.ru>
<CALP_NYQZHLp4+so=JXJwVonP8Rkz2nx4uwRZOP1DXGtY_gW8eQ@mail.gmail.com>
<99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru>
<999caa0e-d015-42da-9b79-001087e719b8@enterprisedb.com>
<0cab2102-4786-416a-ae50-b606a495d01b@enterprisedb.com>
<801646A4-65A3-4DF1-84CD-7BF836E178BD@yandex-team.ru>
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/99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru
From cc87fe5e053c063c98b95abc3d76c432e848d649 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
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 <x4mmm@yandex-team.ru>
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.cindex 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)
Attachments:
[text/plain] v1-0002-pg_basebackup-to-create-replication-slot-early.patch (4.9K, ../985de9f0-cbb6-4235-a6cd-32242f74e1f3@enterprisedb.com/2-v1-0002-pg_basebackup-to-create-replication-slot-early.patch)
download | inline diff:
From cc87fe5e053c063c98b95abc3d76c432e848d649 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
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 <x4mmm@yandex-team.ru>
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.cindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nick.ivanov@enterprisedb.com, x4mmm@yandex-team.ru, alvherre@kurilemu.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Possible race condition in pg_basebackup
In-Reply-To: <985de9f0-cbb6-4235-a6cd-32242f74e1f3@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox