pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed

Possible race condition in pg_basebackup
16+ messages / 4 participants
[nested] [flat]

* Possible race condition in pg_basebackup
@ 2026-08-21 10:30  Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 2 replies; 16+ messages in thread

From: Nick Ivanov @ 2026-08-21 10:30 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

Hello,

We are encountering a possible race condition when executing several
`pg_basebackup --wal-method=stream --slot=... --create-slot` concurrently
while initialising streaming replicas. Our automation starts 3
pg_basebackup processes on 3 future replica servers within 1 second of each
other. One of them almost always fails with "requested WAL segment ... has
already been removed".

wal_keep_size = 0 (the default) on the server.

The server log shows three checkpoints starting within 1 second of each
other, recycling the WAL segment needed by the failing pg_basebackup.

Looking at the pg_basebackup code, we can see that it sends the BASE_BACKUP
command to the server, triggering the checkpoint, before
calling StartLogStreamer() that creates the slot and begins streaming WAL.
The time gap between the BASE_BACKUP and the slot creation seems to be the
reason for the failure we see.

Two obvious workarounds exist: create slots as a separate step before
running pg_basebackup, or update wal_keep_size to some sensible value to
prevent WAL segments from being recycled prematurely.

It seems to me, however, that pg_basebackup can be improved, regardless of
the existing workarounds, to create the slot, if it's being requested,
before starting the actual backup.

I'd like to hear feedback from the community on this.

Thanks

Nick Ivanov
EDB

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-21 11:28  Andrey Borodin <x4mmm@yandex-team.ru>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  1 sibling, 0 replies; 16+ messages in thread

From: Andrey Borodin @ 2026-08-21 11:28 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>



> On 21 Aug 2026, at 13:30, Nick Ivanov <nick.ivanov@enterprisedb.com> wrote:

Hi Nick,

I think your analysis is correct.  More precisely, the problematic
sequence seems to be:

1. The first BASE_BACKUP takes its starting checkpoint and returns its
   REDO location as xlogstart.
2. There is no slot protecting xlogstart yet.
3. Another checkpoint occurs before the slot is created.  In your test it
   is forced by another BASE_BACKUP, but it could also be a scheduled or
   manually requested checkpoint.
4. That checkpoint can recycle the segment containing xlogstart.
5. Only then does the first pg_basebackup enter StartLogStreamer(), create
   its slot with RESERVE_WAL, and request WAL starting at xlogstart.

RESERVE_WAL can only reserve WAL which still exists when the slot is
created.  It cannot retrospectively protect the earlier base-backup start
point.  The active-backup counter established by pg_backup_start() makes
WAL contain the required full-page images; it does not retain WAL
segments.

Interestingly, essentially this race was noticed during the 2017 review
of --create-slot.  Jeff Janes asked whether the slot had to be created
before the checkpoint because streaming starts at the checkpoint's REDO
location [0].  It was thought that a subsequent checkpoint before the WAL
streamer connected was unlikely enough that the concern might be only
theoretical [1].  Your concurrent backups make that subsequent-checkpoint
scenario easy to hit, but the identity of whoever requested the checkpoint
is not important.  A base backup must not depend on no checkpoint occurring
in this interval.

There is also a mismatch with the current documentation, which says that
--create-slot creates the slot "before starting the backup" [2].  In the
current code, pg_basebackup sends BASE_BACKUP and obtains xlogstart before
StartLogStreamer() creates the slot.

I tried a small client-side proof-of-concept which creates the slot before
BASE_BACKUP.  It closes this window, but leaves the slot inactive during
the starting checkpoint, which interacts badly with
idle_replication_slot_timeout.  It also does not protect an existing slot
whose restart_lsn is still NULL.  Therefore, I think this needs a design
discussion rather than just moving client-side calls around.

Perhaps BASE_BACKUP should retain WAL from the exact REDO location of the
checkpoint it selected until the WAL streamer has taken over.  This also
handles backups from a standby, where the selected restartpoint may be
older than the current replay location.  The server knows the exact LSN
and can release such a backup-owned retention horizon on backup completion
or abort, without transferring a temporary slot between the two client
connections.

Pre-creating the physical slots with WAL reserved is a reliable workaround.
A positive wal_keep_size also makes the window less likely, but is a
size-based cushion rather than protection tied to these backups.

As an operational aside, when provisioning several replicas at once I
would normally store one reusable base backup with WAL-G or pgBackRest and
restore it several times.  That avoids making the primary transmit the
same cluster and take several backup checkpoints.  But this does not make
the pg_basebackup race acceptable; I think you found a real bug.

Thank you!


Best regards, Andrey Borodin.

[0] https://www.postgresql.org/message-id/1505248760.16872.1.camel%40credativ.de
[1] https://www.postgresql.org/message-id/CAMkU%3D1z_OucAFdersKxqjKsNUwFCrjvqV0%3Dd-5o9Q17nR53s_g%40mail...
[2] https://www.postgresql.org/docs/current/app-pgbasebackup.html







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-21 16:35  Álvaro Herrera <alvherre@kurilemu.de>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  1 sibling, 2 replies; 16+ messages in thread

From: Álvaro Herrera @ 2026-08-21 16:35 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: pgsql-hackers@lists.postgresql.org

Hello Nick

On 2026-Aug-21, Nick Ivanov wrote:

> We are encountering a possible race condition when executing several
> `pg_basebackup --wal-method=stream --slot=... --create-slot` concurrently
> while initialising streaming replicas. Our automation starts 3
> pg_basebackup processes on 3 future replica servers within 1 second of each
> other. One of them almost always fails with "requested WAL segment ... has
> already been removed".

I think this is related to this thread here:
https://www.postgresql.org/message-id/flat/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj%40aliba...

and to this commit

Author: Amit Kapila <akapila@postgresql.org>
Branch: master Release: REL_19_BR [006dd4b2e] 2025-12-08 05:21:22 +0000
Branch: REL_18_STABLE Release: REL_18_2 [d3ceb2084] 2025-12-08 05:33:14 +0000

    Prevent invalidation of newly created replication slots.
    
    A race condition could cause a newly created replication slot to become
    invalidated between WAL reservation and a checkpoint.
    
    Previously, if the required WAL was removed, we retried the reservation
    process. However, the slot could still be invalidated before the retry if
    the WAL was not yet removed but the checkpoint advanced the redo pointer
    beyond the slot's intended restart LSN and computed the minimum LSN that
    needs to be preserved for the slots.
    
    The fix is to acquire an exclusive lock on ReplicationSlotAllocationLock
    during WAL reservation to serialize WAL reservation and checkpoint's
    minimum restart_lsn computation. This ensures that, if WAL reservation
    occurs first, the checkpoint waits until restart_lsn is updated before
    removing WAL. If the checkpoint runs first, subsequent WAL reservations
    pick a position at or after the latest checkpoint's redo pointer.
    
    We can't use the same fix for branch 17 and prior because commit
    2090edc6f3 changed to compute to the minimum restart_LSN among slot's at
    the beginning of checkpoint (or restart point). The fix for 17 and prior
    branches is under discussion and will be committed separately.
    
    Reported-by: suyu.cmj <mengjuan.cmj@alibaba-inc.com>
    Author: Hou Zhijie <houzj.fnst@fujitsu.com>
    Reviewed-by: Vitaly Davydov <v.davydov@postgrespro.ru>
    Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
    Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
    Backpatch-through: 18
    Discussion: https://postgr.es/m/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com


and to this other commit


Author: Amit Kapila <akapila@postgresql.org>
Branch: REL_17_STABLE Release: REL_17_8 [3510ebeb0] 2026-01-08 07:17:56 +0000
Branch: REL_16_STABLE Release: REL_16_12 [24cce33c3] 2026-01-08 07:07:23 +0000
Branch: REL_15_STABLE Release: REL_15_16 [aae05622a] 2026-01-08 06:54:52 +0000
Branch: REL_14_STABLE Release: REL_14_21 [7406df605] 2026-01-08 06:44:28 +0000

    Prevent invalidation of newly created replication slots.
    
    A race condition could cause a newly created replication slot to become
    invalidated between WAL reservation and a checkpoint.
    
    Previously, if the required WAL was removed, we retried the reservation
    process. However, the slot could still be invalidated before the retry if
    the WAL was not yet removed but the checkpoint advanced the redo pointer
    beyond the slot's intended restart LSN and computed the minimum LSN that
    needs to be preserved for the slots.
    
    The fix is to acquire an exclusive lock on ReplicationSlotAllocationLock
    during WAL reservation, and a shared lock during the minimum LSN
    calculation at checkpoints to serialize the process. This ensures that, if
    WAL reservation occurs first, the checkpoint waits until restart_lsn is
    updated before calculating the minimum LSN. If the checkpoint runs first,
    subsequent WAL reservations pick a position at or after the latest
    checkpoint's redo pointer.
    
    We used a similar fix in HEAD (via commit 006dd4b2e5) and 18. The
    difference is that in 17 and prior branches we need to additionally handle
    the race condition with slot's minimum LSN computation during checkpoints.
    
    Reported-by: suyu.cmj <mengjuan.cmj@alibaba-inc.com>
    Author: Hou Zhijie <houzj.fnst@fujitsu.com>
    Author: vignesh C <vignesh21@gmail.com>
    Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
    Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
    Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
    Backpatch-through: 14
    Discussion: https://postgr.es/m/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com


What version are you using?

If you're using a version that contains these fixes, then we may have
some slightly different bug ...

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No hay ausente sin culpa ni presente sin disculpa" (Prov. francés)






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-24 07:56  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Álvaro Herrera <alvherre@kurilemu.de>
  1 sibling, 0 replies; 16+ messages in thread

From: Nick Ivanov @ 2026-08-24 07:56 UTC (permalink / raw)
  To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: pgsql-hackers@lists.postgresql.org

Hi Álvaro,

Thanks for the info. I did find the thread you are referring to, my
understanding was that the fix was back-ported to 17.8; we are on 17.10. I
will also re-test on the latest 18 version just to be sure, but I think my
situation is different -- the slot here is invalidated _by_ a checkpoint
that recycles some WAL segments when multiple concurrent basebackups
trigger checkpoints.

Cheers

Nick

On Fri, Aug 21, 2026 at 5:35 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

> Hello Nick
>
> On 2026-Aug-21, Nick Ivanov wrote:
>
> > We are encountering a possible race condition when executing several
> > `pg_basebackup --wal-method=stream --slot=... --create-slot` concurrently
> > while initialising streaming replicas. Our automation starts 3
> > pg_basebackup processes on 3 future replica servers within 1 second of
> each
> > other. One of them almost always fails with "requested WAL segment ...
> has
> > already been removed".
>
> I think this is related to this thread here:
>
> https://www.postgresql.org/message-id/flat/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj%40aliba...
>
> and to this commit
>
> Author: Amit Kapila <akapila@postgresql.org>
> Branch: master Release: REL_19_BR [006dd4b2e] 2025-12-08 05:21:22 +0000
> Branch: REL_18_STABLE Release: REL_18_2 [d3ceb2084] 2025-12-08 05:33:14
> +0000
>
>     Prevent invalidation of newly created replication slots.
>
>     A race condition could cause a newly created replication slot to become
>     invalidated between WAL reservation and a checkpoint.
>
>     Previously, if the required WAL was removed, we retried the reservation
>     process. However, the slot could still be invalidated before the retry
> if
>     the WAL was not yet removed but the checkpoint advanced the redo
> pointer
>     beyond the slot's intended restart LSN and computed the minimum LSN
> that
>     needs to be preserved for the slots.
>
>     The fix is to acquire an exclusive lock on
> ReplicationSlotAllocationLock
>     during WAL reservation to serialize WAL reservation and checkpoint's
>     minimum restart_lsn computation. This ensures that, if WAL reservation
>     occurs first, the checkpoint waits until restart_lsn is updated before
>     removing WAL. If the checkpoint runs first, subsequent WAL reservations
>     pick a position at or after the latest checkpoint's redo pointer.
>
>     We can't use the same fix for branch 17 and prior because commit
>     2090edc6f3 changed to compute to the minimum restart_LSN among slot's
> at
>     the beginning of checkpoint (or restart point). The fix for 17 and
> prior
>     branches is under discussion and will be committed separately.
>
>     Reported-by: suyu.cmj <mengjuan.cmj@alibaba-inc.com>
>     Author: Hou Zhijie <houzj.fnst@fujitsu.com>
>     Reviewed-by: Vitaly Davydov <v.davydov@postgrespro.ru>
>     Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
>     Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
>     Backpatch-through: 18
>     Discussion:
> https://postgr.es/m/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com
>
>
> and to this other commit
>
>
> Author: Amit Kapila <akapila@postgresql.org>
> Branch: REL_17_STABLE Release: REL_17_8 [3510ebeb0] 2026-01-08 07:17:56
> +0000
> Branch: REL_16_STABLE Release: REL_16_12 [24cce33c3] 2026-01-08 07:07:23
> +0000
> Branch: REL_15_STABLE Release: REL_15_16 [aae05622a] 2026-01-08 06:54:52
> +0000
> Branch: REL_14_STABLE Release: REL_14_21 [7406df605] 2026-01-08 06:44:28
> +0000
>
>     Prevent invalidation of newly created replication slots.
>
>     A race condition could cause a newly created replication slot to become
>     invalidated between WAL reservation and a checkpoint.
>
>     Previously, if the required WAL was removed, we retried the reservation
>     process. However, the slot could still be invalidated before the retry
> if
>     the WAL was not yet removed but the checkpoint advanced the redo
> pointer
>     beyond the slot's intended restart LSN and computed the minimum LSN
> that
>     needs to be preserved for the slots.
>
>     The fix is to acquire an exclusive lock on
> ReplicationSlotAllocationLock
>     during WAL reservation, and a shared lock during the minimum LSN
>     calculation at checkpoints to serialize the process. This ensures
> that, if
>     WAL reservation occurs first, the checkpoint waits until restart_lsn is
>     updated before calculating the minimum LSN. If the checkpoint runs
> first,
>     subsequent WAL reservations pick a position at or after the latest
>     checkpoint's redo pointer.
>
>     We used a similar fix in HEAD (via commit 006dd4b2e5) and 18. The
>     difference is that in 17 and prior branches we need to additionally
> handle
>     the race condition with slot's minimum LSN computation during
> checkpoints.
>
>     Reported-by: suyu.cmj <mengjuan.cmj@alibaba-inc.com>
>     Author: Hou Zhijie <houzj.fnst@fujitsu.com>
>     Author: vignesh C <vignesh21@gmail.com>
>     Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
>     Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
>     Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
>     Backpatch-through: 14
>     Discussion:
> https://postgr.es/m/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com
>
>
> What version are you using?
>
> If you're using a version that contains these fixes, then we may have
> some slightly different bug ...
>
> --
> Álvaro Herrera        Breisgau, Deutschland  —
> https://www.EnterpriseDB.com/
> "No hay ausente sin culpa ni presente sin disculpa" (Prov. francés)
>


-- 
Nick Ivanov
Solutions Architect
www.enterprisedb.com

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-25 06:32  Andrey Borodin <x4mmm@yandex-team.ru>
  parent: Álvaro Herrera <alvherre@kurilemu.de>
  1 sibling, 1 reply; 16+ messages in thread

From: Andrey Borodin @ 2026-08-25 06:32 UTC (permalink / raw)
  To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Nick Ivanov <nick.ivanov@enterprisedb.com>; pgsql-hackers@lists.postgresql.org

Hi Alvaro,

On Fri, Aug 21, 2026 at 6:35 PM Alvaro Herrera wrote:
> If you're using a version that contains these fixes, then we may have
> some slightly different bug ...

Yes, this is a different bug.  I added a deterministic injection-point test
on current master, which already contains 006dd4b2e.  It stops
BASE_BACKUP after selecting the startpoint but before sending it to the
client, recycles that WAL, and then observes pg_basebackup fail when its WAL
streamer starts.  As a reproducer, the test succeeds only when pg_basebackup
fails with the expected missing-WAL error.

006dd4b2e protects an already created slot while it reserves WAL.  In this
case no slot exists yet, so it cannot protect the earlier backup startpoint.

PFA the test patch.


Best regards, Andrey Borodin.

Attachments:

  [application/octet-stream] v1-0001-Demonstrate-WAL-recycling-race-in-pg_basebackup-cre.patch (6.2K, ../../5516902D-65A5-4C61-8568-E32F111C89EF@yandex-team.ru/2-v1-0001-Demonstrate-WAL-recycling-race-in-pg_basebackup-cre.patch)
  download | inline diff:
From b48a2713483045d88a21e6ea0efd99e4a9cf6343 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Mon, 24 Aug 2026 13:52:58 +0300
Subject: [PATCH] Demonstrate WAL recycling race in pg_basebackup --create-slot

pg_basebackup creates a requested replication slot only after the
server has selected and sent the backup startpoint. Nothing retains WAL
in that window, so a checkpoint can recycle the startpoint segment
before the WAL streamer starts.

Add an injection point before the server sends the selected startpoint
and a TAP test that recycles the segment while the backup waits there,
then observes the resulting missing-WAL failure.
---
 src/backend/backup/basebackup.c               |   6 +
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/056_basebackup_slot_race.pl    | 121 ++++++++++++++++++
 3 files changed, 128 insertions(+)
 create mode 100644 src/test/recovery/t/056_basebackup_slot_race.pl

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index e3c04ecd810..656d4f3aeab 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -324,6 +324,12 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 			state.bytes_total_is_valid = true;
 		}
 
+		/*
+		 * The startpoint has been selected, but the client does not know it
+		 * yet and therefore cannot have created the requested slot.
+		 */
+		INJECTION_POINT("basebackup-before-send-startpoint", NULL);
+
 		/* notify basebackup sink about start of backup */
 		bbsink_begin_backup(sink, &state, SINK_BUFFER_LENGTH);
 
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 39ec8c4946d..b61bc60c000 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -64,6 +64,7 @@ tests += {
       't/053_standby_login_event_trigger.pl',
       't/054_unlogged_sequence_promotion.pl',
       't/055_cascade_reconnect.pl',
+      't/056_basebackup_slot_race.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/056_basebackup_slot_race.pl b/src/test/recovery/t/056_basebackup_slot_race.pl
new file mode 100644
index 00000000000..29da020d762
--- /dev/null
+++ b/src/test/recovery/t/056_basebackup_slot_race.pl
@@ -0,0 +1,121 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Demonstrate WAL recycling before pg_basebackup creates its requested slot.
+#
+# The injection point stops BASE_BACKUP after choosing its startpoint but
+# before sending it to the client.  The test recycles that WAL and then lets
+# pg_basebackup create the slot and start its WAL streamer.  The test passes
+# when pg_basebackup fails with the expected missing-WAL error.
+
+use strict;
+use warnings FATAL => 'all';
+use File::Path qw(rmtree);
+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';
+}
+
+# Small WAL segments make recycling cheap.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 1, extra => [ '--wal-segsize', '1' ]);
+$node->append_conf(
+	'postgresql.conf', q[
+wal_keep_size = 0
+min_wal_size = 2MB
+max_wal_size = 4MB
+checkpoint_timeout = 1h
+]);
+$node->start;
+
+# injection_points may not be installed under installcheck.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Stop BASE_BACKUP before it sends the selected startpoint to the client.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('basebackup-before-send-startpoint', 'wait');"
+);
+
+my $backupdir = $node->backup_dir . '/basebackup_race';
+my ($bb_stdout, $bb_stderr) = ('', '');
+my $bb_timeout =
+  IPC::Run::timeout(3 * $PostgreSQL::Test::Utils::timeout_default);
+my $bb = IPC::Run::start(
+	[
+		'pg_basebackup',
+		'--pgdata' => $backupdir,
+		'--wal-method' => 'stream',
+		'--slot' => 'basebackup_race',
+		'--create-slot',
+		'--checkpoint' => 'fast',
+		'--no-sync',
+		'-d' => $node->connstr('postgres')
+	],
+	'>' => \$bb_stdout,
+	'2>' => \$bb_stderr,
+	$bb_timeout);
+
+$node->wait_for_event('walsender', 'basebackup-before-send-startpoint');
+
+# The client cannot create the slot before it receives the startpoint.
+is( $node->safe_psql(
+		'postgres', 'SELECT count(*) FROM pg_replication_slots;'),
+	'0',
+	'no replication slot exists while startpoint is unprotected');
+
+# do_pg_backup_start() used the current checkpoint's REDO pointer.
+my $startpoint_wal = $node->safe_psql('postgres',
+	'SELECT pg_walfile_name(redo_lsn) FROM pg_control_checkpoint();');
+note "backup startpoint is in WAL segment $startpoint_wal";
+
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint exists while waiting');
+
+# Nothing retains the startpoint while BASE_BACKUP waits.
+$node->advance_wal(10);
+$node->safe_psql('postgres', 'CHECKPOINT;');
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'0',
+	'WAL segment containing the backup startpoint was recycled before slot '
+	  . 'creation'
+);
+
+# Let pg_basebackup create the slot and request the recycled WAL.
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('basebackup-before-send-startpoint');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('basebackup-before-send-startpoint');");
+
+$bb->finish;
+note "pg_basebackup stderr:\n$bb_stderr";
+
+isnt($bb->result(0), 0, 'pg_basebackup failed as expected (bug reproduced)')
+  or diag "pg_basebackup stdout: $bb_stdout\npg_basebackup stderr: $bb_stderr";
+like(
+	$bb_stderr,
+	qr/requested WAL segment [0-9A-F]+ has already been removed/,
+	'WAL streamer failed because the startpoint segment was removed')
+  or diag "pg_basebackup stderr: $bb_stderr";
+
+# The WAL streamer created the slot before noticing the missing segment.
+rmtree($backupdir);
+$node->safe_psql('postgres',
+	"SELECT pg_drop_replication_slot(slot_name) FROM pg_replication_slots "
+	  . "WHERE slot_name = 'basebackup_race';"
+);
+
+done_testing();
-- 
That's all, folks. May the source be with you.


=

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-28 18:05  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Andrey Borodin <x4mmm@yandex-team.ru>
  0 siblings, 1 reply; 16+ messages in thread

From: Nick Ivanov @ 2026-08-28 18:05 UTC (permalink / raw)
  To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers@lists.postgresql.org

Hello,

On Fri, Aug 21, 2026 at 12:28 PM Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

>
>
>  I think this needs a design
> discussion rather than just moving client-side calls around.
>
> Perhaps BASE_BACKUP should retain WAL from the exact REDO location of the
> checkpoint it selected until the WAL streamer has taken over.  This also
> handles backups from a standby, where the selected restartpoint may be
> older than the current replay location.  The server knows the exact LSN
> and can release such a backup-owned retention horizon on backup completion
> or abort, without transferring a temporary slot between the two client
> connections.


The attached patch is intended to fix the WAL recycle race condition
discussed in this thread. It's more to kick off the design discussion, as
suggested by Andrey earlier, than to provide the final answer. It follows
the replication slots example of maintaining a list of in-progress backups
in a shared memory structure and allowing the checkpointer, via KeepLogSeg,
to determine which segments it should not recycle/remove.

The memory structure is much simpler than ReplicationSlot; we don't need to
store any complex state information, and each backup is supposed to have
its unique start XLogRecPtr, which we can use to distinguish between them.
We still introduce a BackupInProgress struct for possible future extension,
instead of using bare XLogRecPtr.

We need to define a new GUC, max_concurrent_backups, to control the size of
the BackupInProgress array.

When the backup starts, it requests a checkpoint and records its start
point in BackupCtlData; when it ends or is aborted, the corresponding entry
is removed. KeepLogSeg checks the oldest XLogRecPtr needed by backups when
deciding what segments need to be removed by a checkpoint.

I retrofitted Andrey Borodin's reproducer test by flipping its
success/failure criterion, and it seems to confirm the fix works. It
doesn't seem to break anything else either.

I left out documentation updates until later; since it's my very first
Postgres patch, its fate is uncertain. I'll be grateful for any feedback.

I did use Claude to vet the design and proofread the resulting code, but
the actual code is all mine.

Cheers

-- 
Nick Ivanov
www.enterprisedb.com

Attachments:

  [application/octet-stream] v1-0001-Fix-WAL-recycle-race-in-pg_basebackup.patch (18.2K, ../../CALP_NYQZHLp4+so=JXJwVonP8Rkz2nx4uwRZOP1DXGtY_gW8eQ@mail.gmail.com/3-v1-0001-Fix-WAL-recycle-race-in-pg_basebackup.patch)
  download | inline diff:
From 05d05bb10767cacb6e9f170c35bbbe62b10c4cc8 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
Date: Fri, 28 Aug 2026 17:58:11 +0100
Subject: [PATCH v1] Fix WAL recycle race in pg_basebackup

Introduce a new shared memory structure to keep the WAL start point for each backup in progress and the corresponding accessor functions to inform the checkpointer of WAL segments we want to keep while the backups are running
---
 src/backend/access/transam/xlog.c             |  23 +++
 src/backend/access/transam/xlogbackup.c       | 148 ++++++++++++++++++
 src/backend/backup/basebackup.c               |   6 +
 .../utils/activity/wait_event_names.txt       |   1 +
 src/backend/utils/misc/guc_parameters.dat     |   8 +
 src/backend/utils/misc/postgresql.conf.sample |   2 +
 src/include/access/xlogbackup.h               |  40 +++++
 src/include/storage/lwlocklist.h              |   1 +
 src/include/storage/subsystemlist.h           |   1 +
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/056_basebackup_slot_race.pl    | 135 ++++++++++++++++
 11 files changed, 366 insertions(+)
 create mode 100644 src/test/recovery/t/056_basebackup_slot_race.pl

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index de4c96e135f..744f1b3dcd3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -58,6 +58,7 @@
 #include "access/xact.h"
 #include "access/xlog_internal.h"
 #include "access/xlogarchive.h"
+#include "access/xlogbackup.h"
 #include "access/xloginsert.h"
 #include "access/xlogreader.h"
 #include "access/xlogrecovery.h"
@@ -8536,6 +8537,7 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	XLogSegNo	currSegNo;
 	XLogSegNo	segno;
 	XLogRecPtr	keep;
+	XLogRecPtr	keep_for_backups;
 
 	XLByteToSeg(recptr, currSegNo, wal_segment_size);
 	segno = currSegNo;
@@ -8543,9 +8545,22 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	/* Calculate how many segments are kept by slots. */
 	keep = XLogGetReplicationSlotMinimumLSN();
 	if (XLogRecPtrIsValid(keep) && keep < recptr)
+		XLByteToSeg(keep, segno, wal_segment_size);
+
+	/*
+	 * Check if we need to keep more segments for in-progress backups.
+	 * This will also be subject to max_slot_wal_keep_size_mb, if set.
+	 */
+	keep_for_backups = GetOldestBackupStartLSN();
+	if (XLogRecPtrIsValid(keep_for_backups) &&
+		(!XLogRecPtrIsValid(keep) || keep_for_backups < keep))
 	{
+		keep = keep_for_backups;
 		XLByteToSeg(keep, segno, wal_segment_size);
+	}
 
+	if (segno < currSegNo)
+	{
 		/*
 		 * Account for max_slot_wal_keep_size to avoid keeping more than
 		 * configured.  However, don't do that during a binary upgrade: if
@@ -9694,6 +9709,8 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			WALInsertLockRelease();
 		} while (!gotUniqueStartpoint);
 
+		RegisterBackupStartpoint(state->startpoint);
+
 		/*
 		 * Construct tablespace_map file.
 		 */
@@ -9898,6 +9915,9 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 
 	WALInsertLockRelease();
 
+	/* Unregister from the shared control structure */
+	UnregisterBackupStartpoint();
+
 	/*
 	 * If we are taking an online backup from the standby, we confirm that the
 	 * standby has not been promoted during the backup.
@@ -10133,6 +10153,9 @@ do_pg_abort_backup(int code, Datum arg)
 		sessionBackupState = SESSION_BACKUP_NONE;
 		WALInsertLockRelease();
 
+		/* Unregister from the shared control structure */
+		UnregisterBackupStartpoint();
+
 		if (!during_backup_start)
 			ereport(WARNING,
 					errmsg("aborting backup due to backend exiting before pg_backup_stop was called"));
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index cf5cc8ead96..1088eb2d949 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -16,6 +16,154 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+
+/* Control array for in-progress backups */
+BackupCtlData *BackupCtl = NULL;
+
+static void BackupCtlShmemRequest(void *arg);
+static void BackupCtlShmemInit(void *arg);
+
+const ShmemCallbacks BackupCtlShmemCallbacks = {
+	.request_fn = BackupCtlShmemRequest,
+	.init_fn = BackupCtlShmemInit,
+};
+
+/* This backend's backup control structure in the shared memory array */
+BackupInProgress *MyBackupInProgress = NULL;
+
+/* GUC */
+int max_concurrent_backups = 10; /* the maximum number of concurrent backups */
+
+/*
+ * Register shared memory space for the backup control structure
+ */
+static void BackupCtlShmemRequest(void *arg)
+{
+	Size size;
+
+	/* max_concurrent_backups is at least 1 */
+	Assert(max_concurrent_backups > 0);
+
+	size = offsetof(BackupCtlData, backups);
+	size = add_size(size, mul_size(max_concurrent_backups, sizeof(BackupInProgress)));
+	ShmemRequestStruct(.name = "Backup Ctl",
+		.size = size,
+		.ptr = (void **)&BackupCtl);
+}
+
+/*
+ * Initialize shared memory for the backup control structure.
+ *
+ * No cleanup is needed on shmem_exit.
+ */
+static void BackupCtlShmemInit(void *arg)
+{
+	int i;
+
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		BackupCtl->backups[i].startpoint = InvalidXLogRecPtr;
+	}
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+}
+
+/*
+ * Register this backup's startpoint.
+ *
+ * We update the oldest startpoint across all in-progress backups here.
+ *
+ */
+
+void
+RegisterBackupStartpoint(XLogRecPtr startpoint)
+{
+	int i;
+
+	Assert(BackupCtl != NULL);
+	Assert(MyBackupInProgress == NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		/* Find the first unused entry */
+		if (BackupCtl->backups[i].startpoint == InvalidXLogRecPtr)
+		{
+			BackupCtl->backups[i].startpoint = startpoint;
+			MyBackupInProgress = &BackupCtl->backups[i];
+			/* Update the oldest startpoint if necessary */
+			if (!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+				startpoint < BackupCtl->oldestStartpoint)
+					BackupCtl->oldestStartpoint = startpoint;
+			break;
+		}
+	}
+	LWLockRelease(BackupControlLock);
+
+	/* If the array is full, bail out */
+	if (i == max_concurrent_backups)
+		ereport(ERROR,
+				(errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+				 errmsg("maximum number of concurrent backups reached"),
+				 errhint("Wait for another backup to finish, or increase max_concurrent_backups.")));
+}
+
+/*
+ * Unregister this backup.
+ *
+ * We also recalculate the oldest startpoint across all remaining
+ * in-progress backups.
+ *
+ */
+
+void
+UnregisterBackupStartpoint(void)
+{
+	XLogRecPtr candidate_startpoint;
+
+	if (MyBackupInProgress == NULL)
+		return;
+
+	Assert(BackupCtl != NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+
+	MyBackupInProgress->startpoint = InvalidXLogRecPtr;
+
+	/* Invalidate the oldest startpoint */
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+	/* Scan the array to find the new oldest startpoint */
+	for (int i = 0; i < max_concurrent_backups; i++)
+	{
+		candidate_startpoint = BackupCtl->backups[i].startpoint;
+		if (XLogRecPtrIsValid(candidate_startpoint) &&
+			(!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+			 candidate_startpoint < BackupCtl->oldestStartpoint))
+			BackupCtl->oldestStartpoint  = candidate_startpoint;
+	}
+
+	LWLockRelease(BackupControlLock);
+
+	MyBackupInProgress = NULL;
+}
+
+/*
+ * Return the precomputed minimum startpoint across all in-progress backups
+ * to use when determining what WAL segments to keep.
+ */
+
+XLogRecPtr
+GetOldestBackupStartLSN(void)
+{
+	XLogRecPtr	retval;
+
+	LWLockAcquire(BackupControlLock, LW_SHARED);
+	retval = BackupCtl->oldestStartpoint;
+	LWLockRelease(BackupControlLock);
+
+	return retval;
+}
 
 /*
  * Build contents for backup_label or backup history file.
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index e3c04ecd810..656d4f3aeab 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -324,6 +324,12 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 			state.bytes_total_is_valid = true;
 		}
 
+		/*
+		 * The startpoint has been selected, but the client does not know it
+		 * yet and therefore cannot have created the requested slot.
+		 */
+		INJECTION_POINT("basebackup-before-send-startpoint", NULL);
+
 		/* notify basebackup sink about start of backup */
 		bbsink_begin_backup(sink, &state, SINK_BUFFER_LENGTH);
 
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 256b3a3c02e..54a13e47782 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -371,6 +371,7 @@ WaitLSN	"Waiting to read or update shared Wait-for-LSN state."
 LogicalDecodingControl	"Waiting to read or update logical decoding status information."
 DataChecksumsWorker	"Waiting for data checksums worker."
 AioWorkerControl	"Waiting to update AIO worker information."
+BackupControl	"Waiting to update basebackup state."
 
 #
 # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3c5e16ad1e7..20dbe3946ec 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -1983,6 +1983,14 @@
   max => 'MAX_BACKENDS',
 },
 
+{ name => 'max_concurrent_backups', type => 'int', context => 'PGC_POSTMASTER', group => 'REPLICATION_SENDING',
+  short_desc => 'Sets the maximum number of simultaneously running basebackups.',
+  variable => 'max_concurrent_backups',
+  boot_val => '10',
+  min => '1',
+  max => 'MAX_BACKENDS',
+},
+
 { name => 'max_connections', type => 'int', context => 'PGC_POSTMASTER', group => 'CONN_AUTH_SETTINGS',
   short_desc => 'Sets the maximum number of concurrent connections.',
   variable => 'MaxConnections',
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e759f06b50f..186a2774ef4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -275,6 +275,8 @@
 #commit_delay = 0                       # range 0-100000, in microseconds
 #commit_siblings = 5                    # range 0-1000
 
+#max_concurrent_backups = 10            # range 1-MAX_BACKENDS  
+
 # - Checkpoints -
 
 #checkpoint_timeout = 5min              # range 30s-1d
diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h
index 2cc2f85d9f0..5d997cbfcd7 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -37,6 +37,46 @@ typedef struct BackupState
 	pg_time_t	stoptime;		/* backup stop time */
 } BackupState;
 
+/*
+ * Shared memory state of a backup in progress. Here we keep track of its
+ * start LSN to ensure checkpoints don't recycle or remove the corresponding
+ * WAL segments until we're done.
+ *
+ * Using a struct instead of a bare XLogRecPtr to allow future extensions.
+ */
+typedef struct BackupInProgress {
+	/*
+	 * Each backup triggers its own checkpoint, so their startpoints are
+	 * guaranteed to be unique, and we can use InvalidXLogRecPtr to indicate
+	 * an available entry
+	 */
+	XLogRecPtr	startpoint;
+} BackupInProgress;
+
+/* Shared memory structure for all in-progress backups
+ *
+ * It is protected by the LWLock BackupControlLock; exclusive
+ * for writers, shared for readers.
+ */
+typedef struct BackupCtlData {
+	/* Minimum startpoint of all in-progress backups */
+	XLogRecPtr			oldestStartpoint;
+	BackupInProgress 	backups[FLEXIBLE_ARRAY_MEMBER];
+} BackupCtlData;
+
+/*
+ * Pointer to shared memory
+ */
+extern PGDLLIMPORT BackupCtlData *BackupCtl;
+extern PGDLLIMPORT BackupInProgress *MyBackupInProgress;
+
+/* GUCs */
+extern PGDLLIMPORT int max_concurrent_backups;
+
+extern void RegisterBackupStartpoint(XLogRecPtr startpoint);
+extern void UnregisterBackupStartpoint(void);
+extern XLogRecPtr GetOldestBackupStartLSN(void);
+
 extern char *build_backup_content(BackupState *state,
 								  bool ishistoryfile);
 
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
index d7eb648bd27..80dfc5775a1 100644
--- a/src/include/storage/lwlocklist.h
+++ b/src/include/storage/lwlocklist.h
@@ -89,6 +89,7 @@ PG_LWLOCK(54, WaitLSN)
 PG_LWLOCK(55, LogicalDecodingControl)
 PG_LWLOCK(56, DataChecksumsWorker)
 PG_LWLOCK(57, AioWorkerControl)
+PG_LWLOCK(58, BackupControl)
 
 /*
  * There also exist several built-in LWLock tranches.  As with the predefined
diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h
index 9ad619080be..d8d1c469226 100644
--- a/src/include/storage/subsystemlist.h
+++ b/src/include/storage/subsystemlist.h
@@ -85,6 +85,7 @@ PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks)
+PG_SHMEM_SUBSYSTEM(BackupCtlShmemCallbacks)
 
 /* AIO subsystem. This delegates to the method-specific callbacks */
 PG_SHMEM_SUBSYSTEM(AioShmemCallbacks)
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 39ec8c4946d..b61bc60c000 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -64,6 +64,7 @@ tests += {
       't/053_standby_login_event_trigger.pl',
       't/054_unlogged_sequence_promotion.pl',
       't/055_cascade_reconnect.pl',
+      't/056_basebackup_slot_race.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/056_basebackup_slot_race.pl b/src/test/recovery/t/056_basebackup_slot_race.pl
new file mode 100644
index 00000000000..2e83f825cae
--- /dev/null
+++ b/src/test/recovery/t/056_basebackup_slot_race.pl
@@ -0,0 +1,135 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify that a base backup's startpoint survives WAL recycling triggered by
+# a concurrent checkpoint, even before pg_basebackup has created its own
+# replication slot.
+#
+# The injection point stops BASE_BACKUP after choosing its startpoint but
+# before sending it to the client.  The test recycles WAL up to and including
+# the startpoint's segment while BASE_BACKUP is paused there, then lets
+# pg_basebackup create its slot and start streaming.  The test passes when
+# the startpoint's WAL segment is still on disk and pg_basebackup succeeds,
+# proving that do_pg_backup_start() protects the segment before the slot
+# exists.
+
+use strict;
+use warnings FATAL => 'all';
+use File::Path qw(rmtree);
+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';
+}
+
+# Small WAL segments make recycling cheap.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 1, extra => [ '--wal-segsize', '1' ]);
+$node->append_conf(
+	'postgresql.conf', q[
+wal_keep_size = 0
+min_wal_size = 2MB
+max_wal_size = 4MB
+checkpoint_timeout = 1h
+]);
+$node->start;
+
+# injection_points may not be installed under installcheck.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Stop BASE_BACKUP before it sends the selected startpoint to the client.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('basebackup-before-send-startpoint', 'wait');"
+);
+
+my $backupdir = $node->backup_dir . '/basebackup_race';
+my ($bb_stdout, $bb_stderr) = ('', '');
+my $bb_timeout =
+  IPC::Run::timeout(3 * $PostgreSQL::Test::Utils::timeout_default);
+my $bb = IPC::Run::start(
+	[
+		'pg_basebackup',
+		'--pgdata' => $backupdir,
+		'--wal-method' => 'stream',
+		'--slot' => 'basebackup_race',
+		'--create-slot',
+		'--checkpoint' => 'fast',
+		'--no-sync',
+		'-d' => $node->connstr('postgres')
+	],
+	'>' => \$bb_stdout,
+	'2>' => \$bb_stderr,
+	$bb_timeout);
+
+$node->wait_for_event('walsender', 'basebackup-before-send-startpoint');
+
+# The client cannot have created its slot yet, since it hasn't received the
+# startpoint; the backup's startpoint is instead protected by the shared
+# in-progress-backup registry at this point.
+is( $node->safe_psql(
+		'postgres', 'SELECT count(*) FROM pg_replication_slots;'),
+	'0',
+	'no replication slot exists yet while startpoint is held only by the '
+	  . 'backup registry');
+
+# do_pg_backup_start() used the current checkpoint's REDO pointer.
+my $startpoint_wal = $node->safe_psql('postgres',
+	'SELECT pg_walfile_name(redo_lsn) FROM pg_control_checkpoint();');
+note "backup startpoint is in WAL segment $startpoint_wal";
+
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint exists while waiting');
+
+# Force enough WAL activity and a checkpoint to make the server want to
+# recycle the startpoint's segment.  The backup registry should keep it
+# around anyway, even though no replication slot protects it yet.
+$node->advance_wal(10);
+$node->safe_psql('postgres', 'CHECKPOINT;');
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint survives a concurrent '
+	  . 'checkpoint'
+);
+
+# Let pg_basebackup create the slot and request WAL starting at the
+# (still-present) startpoint.
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('basebackup-before-send-startpoint');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('basebackup-before-send-startpoint');");
+
+$bb->finish;
+note "pg_basebackup stderr:\n$bb_stderr";
+
+is($bb->result(0), 0, 'pg_basebackup succeeded despite concurrent WAL recycling')
+  or diag "pg_basebackup stdout: $bb_stdout\npg_basebackup stderr: $bb_stderr";
+
+# The slot requested via --create-slot should now exist.
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_replication_slots WHERE slot_name = "
+		  . "'basebackup_race';"),
+	'1',
+	'replication slot was created once pg_basebackup received the startpoint'
+);
+
+rmtree($backupdir);
+$node->safe_psql('postgres',
+	"SELECT pg_drop_replication_slot(slot_name) FROM pg_replication_slots "
+	  . "WHERE slot_name = 'basebackup_race';"
+);
+
+done_testing();
-- 
2.50.1 (Apple Git-155)



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-29 14:18  Andrey Borodin <x4mmm@yandex-team.ru>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Andrey Borodin @ 2026-08-29 14:18 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hi Nick,

Thank you for working on this.  I took a look into the patch.

RegisterBackupStartpoint() is called after the starting checkpoint has
selected the startpoint.  Another checkpoint can remove WAL between reading
ControlFile and registering that LSN.  The test stops later, after
registration, so it does not exercise this window.

To test it, I would move the injection point from perform_base_backup() to
immediately before RegisterBackupStartpoint(state->startpoint) in
do_pg_backup_start().  While stopped there, remember the selected segment,
generate WAL and run CHECKPOINT.  I expect the current patch to lose that
segment.

Avoiding this requires registering a conservative current insert or replay
position before requesting the starting checkpoint.  If the selected
startpoint is older, as can happen on a standby, the horizon then has to be
lowered.  Alternatively, selection and registration need an interlock with
WAL removal.

For a backpatch, I think a new postmaster GUC is a non-starter.  It adds a
user-visible limit and shared-memory sizing decisions to a minor-version bug
fix, while concurrent BASE_BACKUP sessions are already bounded by
max_wal_senders.  Unconditionally registering in do_pg_backup_start() also
changes SQL-level pg_backup_start().

I suggested server-owned retention upthread, but after reading the patch I
think your original client-side proposal deserves another look.  Creating
the requested slot before sending BASE_BACKUP directly fixes the reported
--create-slot case, is much smaller to backpatch, and also works when a new
pg_basebackup connects to an older server.  Existing slots with a NULL
restart_lsn, idle_replication_slot_timeout, and server-fetched WAL can be
treated as separate server-side problems.  WDYT?

The September Commitfest is open for registration until September 1.  I
suggest registering the patch now so that further versions and discussion do
not fall between CommitFests.

Thank you!


Best regards, Andrey Borodin.







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-08-31 12:32  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Andrey Borodin <x4mmm@yandex-team.ru>
  0 siblings, 1 reply; 16+ messages in thread

From: Nick Ivanov @ 2026-08-31 12:32 UTC (permalink / raw)
  To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hello Andrey,

Thank you for your comments, they are very helpful.

On 29/08/2026 15:18, Andrey Borodin wrote:
> RegisterBackupStartpoint() is called after the starting checkpoint has
> selected the startpoint.  Another checkpoint can remove WAL between reading
> ControlFile and registering that LSN.  The test stops later, after
> registration, so it does not exercise this window.
> ...
> Avoiding this requires registering a conservative current insert or replay
> position before requesting the starting checkpoint.  If the selected
> startpoint is older, as can happen on a standby, the horizon then has to be
> lowered.  Alternatively, selection and registration need an interlock with
> WAL removal.
I did consider this gap, but thought it wasn't wide enough to be of 
great risk. However, you're right in that, when fixing a race condition 
we should close the gap completely and not resort to a half-measure. I 
will update the test and see where it takes me.
> For a backpatch, I think a new postmaster GUC is a non-starter.  It adds a
> user-visible limit and shared-memory sizing decisions to a minor-version bug
> fix, while concurrent BASE_BACKUP sessions are already bounded by
> max_wal_senders.  Unconditionally registering in do_pg_backup_start() also
> changes SQL-level pg_backup_start().
I didn't like introducing a new GUC myself, but I thought recycling 
max_wal_senders (or max_replication_slots or whatever) would be 
confusing. I believe this race condition can affect `pg_basebackup 
--wal-method=fetch` as well as `--wal-method=stream`, so tying this to 
replication seemed wrong to me. On the other hand, I didn't fully 
consider back-porting the patch, so this needs more thought obviously.
> I suggested server-owned retention upthread, but after reading the patch I
> think your original client-side proposal deserves another look.  Creating
> the requested slot before sending BASE_BACKUP directly fixes the reported
> --create-slot case, is much smaller to backpatch, and also works when a new
> pg_basebackup connects to an older server.  Existing slots with a NULL
> restart_lsn, idle_replication_slot_timeout, and server-fetched WAL can be
> treated as separate server-side problems.  WDYT?
I suspect this won't address the `--wal-method=fetch` situation, which I 
think is subject to the same WAL removal/recycling risk.
> The September Commitfest is open for registration until September 1.  I
> suggest registering the patch now so that further versions and discussion do
> not fall between CommitFests.

Thanks for the advice, will do.

Cheers

Nick







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-07 14:19  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Nick Ivanov @ 2026-09-07 14:19 UTC (permalink / raw)
  To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hello, I'm attaching an updated patch. As suggested, I now attempt to 
reserve the WAL earlier in the process, before the call to 
do_pg_backup_start(), and I moved the injection point closer towards to 
make the test stricter towards the race window. As a side effect, WAL 
reservation behaviour change won't apply when the SQL interface 
(pg_backup_start()) is invoked, which was highlighted earlier.

Since the bug I intend to fix here has simple workarounds, I don't think 
it's necessary to consider backporting the patch to earlier versions, 
and the introduction of a new GUC should therefore be acceptable.

The patch is rebased on top of the master as of approximately 2026-09-07 
14:50:00 UTC.

I agree that a separate patch for pg_basebackup, to make it request the 
slot creation earlier, would be beneficial, and make a separate patch 
for that.


Cheers

Nick

On 31/08/2026 13:32, Nick Ivanov wrote:
> Hello Andrey,
>
> Thank you for your comments, they are very helpful.
>
> On 29/08/2026 15:18, Andrey Borodin wrote:
>> RegisterBackupStartpoint() is called after the starting checkpoint has
>> selected the startpoint.  Another checkpoint can remove WAL between 
>> reading
>> ControlFile and registering that LSN.  The test stops later, after
>> registration, so it does not exercise this window.
>> ...
>> Avoiding this requires registering a conservative current insert or 
>> replay
>> position before requesting the starting checkpoint.  If the selected
>> startpoint is older, as can happen on a standby, the horizon then has 
>> to be
>> lowered.  Alternatively, selection and registration need an interlock 
>> with
>> WAL removal.
> I did consider this gap, but thought it wasn't wide enough to be of 
> great risk. However, you're right in that, when fixing a race 
> condition we should close the gap completely and not resort to a 
> half-measure. I will update the test and see where it takes me. 
>> For a backpatch, I think a new postmaster GUC is a non-starter.  It 
>> adds a
>> user-visible limit and shared-memory sizing decisions to a 
>> minor-version bug
>> fix, while concurrent BASE_BACKUP sessions are already bounded by
>> max_wal_senders.  Unconditionally registering in do_pg_backup_start() 
>> also
>> changes SQL-level pg_backup_start().
> I didn't like introducing a new GUC myself, but I thought recycling 
> max_wal_senders (or max_replication_slots or whatever) would be 
> confusing. I believe this race condition can affect `pg_basebackup 
> --wal-method=fetch` as well as `--wal-method=stream`, so tying this to 
> replication seemed wrong to me. On the other hand, I didn't fully 
> consider back-porting the patch, so this needs more thought obviously.
>> I suggested server-owned retention upthread, but after reading the 
>> patch I
>> think your original client-side proposal deserves another look. Creating
>> the requested slot before sending BASE_BACKUP directly fixes the 
>> reported
>> --create-slot case, is much smaller to backpatch, and also works when 
>> a new
>> pg_basebackup connects to an older server.  Existing slots with a NULL
>> restart_lsn, idle_replication_slot_timeout, and server-fetched WAL 
>> can be
>> treated as separate server-side problems.  WDYT?
> I suspect this won't address the `--wal-method=fetch` situation, which 
> I think is subject to the same WAL removal/recycling risk.
>
From 8d6a26b5d271bab4627d3be28ddaf28536cb7a62 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
Date: Sun, 6 Sep 2026 13:31:12 +0100
Subject: [PATCH v2] Fix WAL recycle race in pg_basebackup

Introduce a new shared memory structure to keep the WAL start point for each backup in progress and the corresponding accessor functions to inform the checkpointer of WAL segments we want to keep while the backups are running. Reserve the WAL early in the process, before invoking do_pg_backup_start(), to ensure it happens before the checkpoint.
---
 src/backend/access/transam/xlog.c             |  40 +++++
 src/backend/access/transam/xlogbackup.c       | 148 ++++++++++++++++++
 src/backend/backup/basebackup.c               |   9 ++
 .../utils/activity/wait_event_names.txt       |   1 +
 src/backend/utils/misc/guc_parameters.dat     |   8 +
 src/backend/utils/misc/postgresql.conf.sample |   2 +
 src/include/access/xlogbackup.h               |  40 +++++
 src/include/storage/lwlocklist.h              |   1 +
 src/include/storage/subsystemlist.h           |   1 +
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/057_basebackup_slot_race.pl    | 135 ++++++++++++++++
 11 files changed, 386 insertions(+)
 create mode 100644 src/test/recovery/t/057_basebackup_slot_race.pl

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3203f2fd4ee..3c878fd2b70 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -58,6 +58,7 @@
 #include "access/xact.h"
 #include "access/xlog_internal.h"
 #include "access/xlogarchive.h"
+#include "access/xlogbackup.h"
 #include "access/xloginsert.h"
 #include "access/xlogreader.h"
 #include "access/xlogrecovery.h"
@@ -8537,6 +8538,7 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	XLogSegNo	currSegNo;
 	XLogSegNo	segno;
 	XLogRecPtr	keep;
+	XLogRecPtr	keep_for_backups;
 
 	XLByteToSeg(recptr, currSegNo, wal_segment_size);
 	segno = currSegNo;
@@ -8544,9 +8546,22 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	/* Calculate how many segments are kept by slots. */
 	keep = XLogGetReplicationSlotMinimumLSN();
 	if (XLogRecPtrIsValid(keep) && keep < recptr)
+		XLByteToSeg(keep, segno, wal_segment_size);
+
+	/*
+	 * Check if we need to keep more segments for in-progress backups.
+	 * This will also be subject to max_slot_wal_keep_size_mb, if set.
+	 */
+	keep_for_backups = GetOldestBackupStartLSN();
+	if (XLogRecPtrIsValid(keep_for_backups) &&
+		(!XLogRecPtrIsValid(keep) || keep_for_backups < keep))
 	{
+		keep = keep_for_backups;
 		XLByteToSeg(keep, segno, wal_segment_size);
+	}
 
+	if (segno < currSegNo)
+	{
 		/*
 		 * Account for max_slot_wal_keep_size to avoid keeping more than
 		 * configured.  However, don't do that during a binary upgrade: if
@@ -9695,6 +9710,25 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			WALInsertLockRelease();
 		} while (!gotUniqueStartpoint);
 
+		/*
+		 * The startpoint has been selected, but the client does not know it
+		 * yet and therefore cannot have created the requested slot.
+		 */
+		INJECTION_POINT("basebackup-before-send-startpoint", NULL);
+
+		/*
+		 * If we are invoked by basebackup, it may have already reserved a WAL
+		 * start point. If that is newer than what's selected here, update
+		 * the reservation.
+		 */
+		if (MyBackupInProgress != NULL &&
+			XLogRecPtrIsValid(MyBackupInProgress->startpoint) &&
+			state->startpoint < MyBackupInProgress->startpoint)
+		{
+				UnregisterBackupStartpoint();
+				RegisterBackupStartpoint(state->startpoint);
+		}
+
 		/*
 		 * Construct tablespace_map file.
 		 */
@@ -9899,6 +9933,9 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 
 	WALInsertLockRelease();
 
+	/* Unregister from the shared control structure */
+	UnregisterBackupStartpoint();
+
 	/*
 	 * If we are taking an online backup from the standby, we confirm that the
 	 * standby has not been promoted during the backup.
@@ -10134,6 +10171,9 @@ do_pg_abort_backup(int code, Datum arg)
 		sessionBackupState = SESSION_BACKUP_NONE;
 		WALInsertLockRelease();
 
+		/* Unregister from the shared control structure */
+		UnregisterBackupStartpoint();
+
 		if (!during_backup_start)
 			ereport(WARNING,
 					errmsg("aborting backup due to backend exiting before pg_backup_stop was called"));
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index cf5cc8ead96..1088eb2d949 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -16,6 +16,154 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+
+/* Control array for in-progress backups */
+BackupCtlData *BackupCtl = NULL;
+
+static void BackupCtlShmemRequest(void *arg);
+static void BackupCtlShmemInit(void *arg);
+
+const ShmemCallbacks BackupCtlShmemCallbacks = {
+	.request_fn = BackupCtlShmemRequest,
+	.init_fn = BackupCtlShmemInit,
+};
+
+/* This backend's backup control structure in the shared memory array */
+BackupInProgress *MyBackupInProgress = NULL;
+
+/* GUC */
+int max_concurrent_backups = 10; /* the maximum number of concurrent backups */
+
+/*
+ * Register shared memory space for the backup control structure
+ */
+static void BackupCtlShmemRequest(void *arg)
+{
+	Size size;
+
+	/* max_concurrent_backups is at least 1 */
+	Assert(max_concurrent_backups > 0);
+
+	size = offsetof(BackupCtlData, backups);
+	size = add_size(size, mul_size(max_concurrent_backups, sizeof(BackupInProgress)));
+	ShmemRequestStruct(.name = "Backup Ctl",
+		.size = size,
+		.ptr = (void **)&BackupCtl);
+}
+
+/*
+ * Initialize shared memory for the backup control structure.
+ *
+ * No cleanup is needed on shmem_exit.
+ */
+static void BackupCtlShmemInit(void *arg)
+{
+	int i;
+
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		BackupCtl->backups[i].startpoint = InvalidXLogRecPtr;
+	}
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+}
+
+/*
+ * Register this backup's startpoint.
+ *
+ * We update the oldest startpoint across all in-progress backups here.
+ *
+ */
+
+void
+RegisterBackupStartpoint(XLogRecPtr startpoint)
+{
+	int i;
+
+	Assert(BackupCtl != NULL);
+	Assert(MyBackupInProgress == NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		/* Find the first unused entry */
+		if (BackupCtl->backups[i].startpoint == InvalidXLogRecPtr)
+		{
+			BackupCtl->backups[i].startpoint = startpoint;
+			MyBackupInProgress = &BackupCtl->backups[i];
+			/* Update the oldest startpoint if necessary */
+			if (!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+				startpoint < BackupCtl->oldestStartpoint)
+					BackupCtl->oldestStartpoint = startpoint;
+			break;
+		}
+	}
+	LWLockRelease(BackupControlLock);
+
+	/* If the array is full, bail out */
+	if (i == max_concurrent_backups)
+		ereport(ERROR,
+				(errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+				 errmsg("maximum number of concurrent backups reached"),
+				 errhint("Wait for another backup to finish, or increase max_concurrent_backups.")));
+}
+
+/*
+ * Unregister this backup.
+ *
+ * We also recalculate the oldest startpoint across all remaining
+ * in-progress backups.
+ *
+ */
+
+void
+UnregisterBackupStartpoint(void)
+{
+	XLogRecPtr candidate_startpoint;
+
+	if (MyBackupInProgress == NULL)
+		return;
+
+	Assert(BackupCtl != NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+
+	MyBackupInProgress->startpoint = InvalidXLogRecPtr;
+
+	/* Invalidate the oldest startpoint */
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+	/* Scan the array to find the new oldest startpoint */
+	for (int i = 0; i < max_concurrent_backups; i++)
+	{
+		candidate_startpoint = BackupCtl->backups[i].startpoint;
+		if (XLogRecPtrIsValid(candidate_startpoint) &&
+			(!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+			 candidate_startpoint < BackupCtl->oldestStartpoint))
+			BackupCtl->oldestStartpoint  = candidate_startpoint;
+	}
+
+	LWLockRelease(BackupControlLock);
+
+	MyBackupInProgress = NULL;
+}
+
+/*
+ * Return the precomputed minimum startpoint across all in-progress backups
+ * to use when determining what WAL segments to keep.
+ */
+
+XLogRecPtr
+GetOldestBackupStartLSN(void)
+{
+	XLogRecPtr	retval;
+
+	LWLockAcquire(BackupControlLock, LW_SHARED);
+	retval = BackupCtl->oldestStartpoint;
+	LWLockRelease(BackupControlLock);
+
+	return retval;
+}
 
 /*
  * Build contents for backup_label or backup history file.
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index e3c04ecd810..8bdbb355b7f 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -18,6 +18,7 @@
 
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "access/xlogrecovery.h"
 #include "backup/backup_manifest.h"
 #include "backup/basebackup.h"
 #include "backup/basebackup_incremental.h"
@@ -244,6 +245,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 {
 	bbsink_state state;
 	XLogRecPtr	endptr;
+	XLogRecPtr	reserve_ptr;
 	TimeLineID	endtli;
 	backup_manifest_info manifest;
 	BackupState *backup_state;
@@ -264,6 +266,13 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 
 	backup_started_in_recovery = RecoveryInProgress();
 
+	/* Reserve the XLog before starting the backup */
+	if (backup_started_in_recovery)
+		reserve_ptr = GetXLogReplayRecPtr(NULL);
+	else
+		reserve_ptr = GetXLogInsertRecPtr();
+	RegisterBackupStartpoint(reserve_ptr);
+
 	InitializeBackupManifest(&manifest, opt->manifest,
 							 opt->manifest_checksum_type);
 
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 0a70cffa081..bb01fcf7f1a 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -371,6 +371,7 @@ WaitLSN	"Waiting to read or update shared Wait-for-LSN state."
 LogicalDecodingControl	"Waiting to read or update logical decoding status information."
 DataChecksumsWorker	"Waiting for data checksums worker."
 AioWorkerControl	"Waiting to update AIO worker information."
+BackupControl	"Waiting to update basebackup state."
 
 #
 # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3c5e16ad1e7..20dbe3946ec 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -1983,6 +1983,14 @@
   max => 'MAX_BACKENDS',
 },
 
+{ name => 'max_concurrent_backups', type => 'int', context => 'PGC_POSTMASTER', group => 'REPLICATION_SENDING',
+  short_desc => 'Sets the maximum number of simultaneously running basebackups.',
+  variable => 'max_concurrent_backups',
+  boot_val => '10',
+  min => '1',
+  max => 'MAX_BACKENDS',
+},
+
 { name => 'max_connections', type => 'int', context => 'PGC_POSTMASTER', group => 'CONN_AUTH_SETTINGS',
   short_desc => 'Sets the maximum number of concurrent connections.',
   variable => 'MaxConnections',
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e759f06b50f..186a2774ef4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -275,6 +275,8 @@
 #commit_delay = 0                       # range 0-100000, in microseconds
 #commit_siblings = 5                    # range 0-1000
 
+#max_concurrent_backups = 10            # range 1-MAX_BACKENDS  
+
 # - Checkpoints -
 
 #checkpoint_timeout = 5min              # range 30s-1d
diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h
index 2cc2f85d9f0..5d997cbfcd7 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -37,6 +37,46 @@ typedef struct BackupState
 	pg_time_t	stoptime;		/* backup stop time */
 } BackupState;
 
+/*
+ * Shared memory state of a backup in progress. Here we keep track of its
+ * start LSN to ensure checkpoints don't recycle or remove the corresponding
+ * WAL segments until we're done.
+ *
+ * Using a struct instead of a bare XLogRecPtr to allow future extensions.
+ */
+typedef struct BackupInProgress {
+	/*
+	 * Each backup triggers its own checkpoint, so their startpoints are
+	 * guaranteed to be unique, and we can use InvalidXLogRecPtr to indicate
+	 * an available entry
+	 */
+	XLogRecPtr	startpoint;
+} BackupInProgress;
+
+/* Shared memory structure for all in-progress backups
+ *
+ * It is protected by the LWLock BackupControlLock; exclusive
+ * for writers, shared for readers.
+ */
+typedef struct BackupCtlData {
+	/* Minimum startpoint of all in-progress backups */
+	XLogRecPtr			oldestStartpoint;
+	BackupInProgress 	backups[FLEXIBLE_ARRAY_MEMBER];
+} BackupCtlData;
+
+/*
+ * Pointer to shared memory
+ */
+extern PGDLLIMPORT BackupCtlData *BackupCtl;
+extern PGDLLIMPORT BackupInProgress *MyBackupInProgress;
+
+/* GUCs */
+extern PGDLLIMPORT int max_concurrent_backups;
+
+extern void RegisterBackupStartpoint(XLogRecPtr startpoint);
+extern void UnregisterBackupStartpoint(void);
+extern XLogRecPtr GetOldestBackupStartLSN(void);
+
 extern char *build_backup_content(BackupState *state,
 								  bool ishistoryfile);
 
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
index d7eb648bd27..80dfc5775a1 100644
--- a/src/include/storage/lwlocklist.h
+++ b/src/include/storage/lwlocklist.h
@@ -89,6 +89,7 @@ PG_LWLOCK(54, WaitLSN)
 PG_LWLOCK(55, LogicalDecodingControl)
 PG_LWLOCK(56, DataChecksumsWorker)
 PG_LWLOCK(57, AioWorkerControl)
+PG_LWLOCK(58, BackupControl)
 
 /*
  * There also exist several built-in LWLock tranches.  As with the predefined
diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h
index 9ad619080be..d8d1c469226 100644
--- a/src/include/storage/subsystemlist.h
+++ b/src/include/storage/subsystemlist.h
@@ -85,6 +85,7 @@ PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks)
+PG_SHMEM_SUBSYSTEM(BackupCtlShmemCallbacks)
 
 /* AIO subsystem. This delegates to the method-specific callbacks */
 PG_SHMEM_SUBSYSTEM(AioShmemCallbacks)
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 72113c5ac6e..db045f9cc1f 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -65,6 +65,7 @@ tests += {
       't/054_unlogged_sequence_promotion.pl',
       't/055_cascade_reconnect.pl',
       't/056_standby_snapshot_export.pl',
+      't/057_basebackup_slot_race.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/057_basebackup_slot_race.pl b/src/test/recovery/t/057_basebackup_slot_race.pl
new file mode 100644
index 00000000000..2e83f825cae
--- /dev/null
+++ b/src/test/recovery/t/057_basebackup_slot_race.pl
@@ -0,0 +1,135 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify that a base backup's startpoint survives WAL recycling triggered by
+# a concurrent checkpoint, even before pg_basebackup has created its own
+# replication slot.
+#
+# The injection point stops BASE_BACKUP after choosing its startpoint but
+# before sending it to the client.  The test recycles WAL up to and including
+# the startpoint's segment while BASE_BACKUP is paused there, then lets
+# pg_basebackup create its slot and start streaming.  The test passes when
+# the startpoint's WAL segment is still on disk and pg_basebackup succeeds,
+# proving that do_pg_backup_start() protects the segment before the slot
+# exists.
+
+use strict;
+use warnings FATAL => 'all';
+use File::Path qw(rmtree);
+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';
+}
+
+# Small WAL segments make recycling cheap.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 1, extra => [ '--wal-segsize', '1' ]);
+$node->append_conf(
+	'postgresql.conf', q[
+wal_keep_size = 0
+min_wal_size = 2MB
+max_wal_size = 4MB
+checkpoint_timeout = 1h
+]);
+$node->start;
+
+# injection_points may not be installed under installcheck.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Stop BASE_BACKUP before it sends the selected startpoint to the client.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('basebackup-before-send-startpoint', 'wait');"
+);
+
+my $backupdir = $node->backup_dir . '/basebackup_race';
+my ($bb_stdout, $bb_stderr) = ('', '');
+my $bb_timeout =
+  IPC::Run::timeout(3 * $PostgreSQL::Test::Utils::timeout_default);
+my $bb = IPC::Run::start(
+	[
+		'pg_basebackup',
+		'--pgdata' => $backupdir,
+		'--wal-method' => 'stream',
+		'--slot' => 'basebackup_race',
+		'--create-slot',
+		'--checkpoint' => 'fast',
+		'--no-sync',
+		'-d' => $node->connstr('postgres')
+	],
+	'>' => \$bb_stdout,
+	'2>' => \$bb_stderr,
+	$bb_timeout);
+
+$node->wait_for_event('walsender', 'basebackup-before-send-startpoint');
+
+# The client cannot have created its slot yet, since it hasn't received the
+# startpoint; the backup's startpoint is instead protected by the shared
+# in-progress-backup registry at this point.
+is( $node->safe_psql(
+		'postgres', 'SELECT count(*) FROM pg_replication_slots;'),
+	'0',
+	'no replication slot exists yet while startpoint is held only by the '
+	  . 'backup registry');
+
+# do_pg_backup_start() used the current checkpoint's REDO pointer.
+my $startpoint_wal = $node->safe_psql('postgres',
+	'SELECT pg_walfile_name(redo_lsn) FROM pg_control_checkpoint();');
+note "backup startpoint is in WAL segment $startpoint_wal";
+
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint exists while waiting');
+
+# Force enough WAL activity and a checkpoint to make the server want to
+# recycle the startpoint's segment.  The backup registry should keep it
+# around anyway, even though no replication slot protects it yet.
+$node->advance_wal(10);
+$node->safe_psql('postgres', 'CHECKPOINT;');
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint survives a concurrent '
+	  . 'checkpoint'
+);
+
+# Let pg_basebackup create the slot and request WAL starting at the
+# (still-present) startpoint.
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('basebackup-before-send-startpoint');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('basebackup-before-send-startpoint');");
+
+$bb->finish;
+note "pg_basebackup stderr:\n$bb_stderr";
+
+is($bb->result(0), 0, 'pg_basebackup succeeded despite concurrent WAL recycling')
+  or diag "pg_basebackup stdout: $bb_stdout\npg_basebackup stderr: $bb_stderr";
+
+# The slot requested via --create-slot should now exist.
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_replication_slots WHERE slot_name = "
+		  . "'basebackup_race';"),
+	'1',
+	'replication slot was created once pg_basebackup received the startpoint'
+);
+
+rmtree($backupdir);
+$node->safe_psql('postgres',
+	"SELECT pg_drop_replication_slot(slot_name) FROM pg_replication_slots "
+	  . "WHERE slot_name = 'basebackup_race';"
+);
+
+done_testing();
-- 
2.50.1 (Apple Git-155)



Attachments:

  [text/plain] v2-0001-Fix-WAL-recycle-race-in-pg_basebackup.patch (19.3K, ../../0cab2102-4786-416a-ae50-b606a495d01b@enterprisedb.com/2-v2-0001-Fix-WAL-recycle-race-in-pg_basebackup.patch)
  download | inline diff:
From 8d6a26b5d271bab4627d3be28ddaf28536cb7a62 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
Date: Sun, 6 Sep 2026 13:31:12 +0100
Subject: [PATCH v2] Fix WAL recycle race in pg_basebackup

Introduce a new shared memory structure to keep the WAL start point for each backup in progress and the corresponding accessor functions to inform the checkpointer of WAL segments we want to keep while the backups are running. Reserve the WAL early in the process, before invoking do_pg_backup_start(), to ensure it happens before the checkpoint.
---
 src/backend/access/transam/xlog.c             |  40 +++++
 src/backend/access/transam/xlogbackup.c       | 148 ++++++++++++++++++
 src/backend/backup/basebackup.c               |   9 ++
 .../utils/activity/wait_event_names.txt       |   1 +
 src/backend/utils/misc/guc_parameters.dat     |   8 +
 src/backend/utils/misc/postgresql.conf.sample |   2 +
 src/include/access/xlogbackup.h               |  40 +++++
 src/include/storage/lwlocklist.h              |   1 +
 src/include/storage/subsystemlist.h           |   1 +
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/057_basebackup_slot_race.pl    | 135 ++++++++++++++++
 11 files changed, 386 insertions(+)
 create mode 100644 src/test/recovery/t/057_basebackup_slot_race.pl

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3203f2fd4ee..3c878fd2b70 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -58,6 +58,7 @@
 #include "access/xact.h"
 #include "access/xlog_internal.h"
 #include "access/xlogarchive.h"
+#include "access/xlogbackup.h"
 #include "access/xloginsert.h"
 #include "access/xlogreader.h"
 #include "access/xlogrecovery.h"
@@ -8537,6 +8538,7 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	XLogSegNo	currSegNo;
 	XLogSegNo	segno;
 	XLogRecPtr	keep;
+	XLogRecPtr	keep_for_backups;
 
 	XLByteToSeg(recptr, currSegNo, wal_segment_size);
 	segno = currSegNo;
@@ -8544,9 +8546,22 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
 	/* Calculate how many segments are kept by slots. */
 	keep = XLogGetReplicationSlotMinimumLSN();
 	if (XLogRecPtrIsValid(keep) && keep < recptr)
+		XLByteToSeg(keep, segno, wal_segment_size);
+
+	/*
+	 * Check if we need to keep more segments for in-progress backups.
+	 * This will also be subject to max_slot_wal_keep_size_mb, if set.
+	 */
+	keep_for_backups = GetOldestBackupStartLSN();
+	if (XLogRecPtrIsValid(keep_for_backups) &&
+		(!XLogRecPtrIsValid(keep) || keep_for_backups < keep))
 	{
+		keep = keep_for_backups;
 		XLByteToSeg(keep, segno, wal_segment_size);
+	}
 
+	if (segno < currSegNo)
+	{
 		/*
 		 * Account for max_slot_wal_keep_size to avoid keeping more than
 		 * configured.  However, don't do that during a binary upgrade: if
@@ -9695,6 +9710,25 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			WALInsertLockRelease();
 		} while (!gotUniqueStartpoint);
 
+		/*
+		 * The startpoint has been selected, but the client does not know it
+		 * yet and therefore cannot have created the requested slot.
+		 */
+		INJECTION_POINT("basebackup-before-send-startpoint", NULL);
+
+		/*
+		 * If we are invoked by basebackup, it may have already reserved a WAL
+		 * start point. If that is newer than what's selected here, update
+		 * the reservation.
+		 */
+		if (MyBackupInProgress != NULL &&
+			XLogRecPtrIsValid(MyBackupInProgress->startpoint) &&
+			state->startpoint < MyBackupInProgress->startpoint)
+		{
+				UnregisterBackupStartpoint();
+				RegisterBackupStartpoint(state->startpoint);
+		}
+
 		/*
 		 * Construct tablespace_map file.
 		 */
@@ -9899,6 +9933,9 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 
 	WALInsertLockRelease();
 
+	/* Unregister from the shared control structure */
+	UnregisterBackupStartpoint();
+
 	/*
 	 * If we are taking an online backup from the standby, we confirm that the
 	 * standby has not been promoted during the backup.
@@ -10134,6 +10171,9 @@ do_pg_abort_backup(int code, Datum arg)
 		sessionBackupState = SESSION_BACKUP_NONE;
 		WALInsertLockRelease();
 
+		/* Unregister from the shared control structure */
+		UnregisterBackupStartpoint();
+
 		if (!during_backup_start)
 			ereport(WARNING,
 					errmsg("aborting backup due to backend exiting before pg_backup_stop was called"));
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index cf5cc8ead96..1088eb2d949 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -16,6 +16,154 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+
+/* Control array for in-progress backups */
+BackupCtlData *BackupCtl = NULL;
+
+static void BackupCtlShmemRequest(void *arg);
+static void BackupCtlShmemInit(void *arg);
+
+const ShmemCallbacks BackupCtlShmemCallbacks = {
+	.request_fn = BackupCtlShmemRequest,
+	.init_fn = BackupCtlShmemInit,
+};
+
+/* This backend's backup control structure in the shared memory array */
+BackupInProgress *MyBackupInProgress = NULL;
+
+/* GUC */
+int max_concurrent_backups = 10; /* the maximum number of concurrent backups */
+
+/*
+ * Register shared memory space for the backup control structure
+ */
+static void BackupCtlShmemRequest(void *arg)
+{
+	Size size;
+
+	/* max_concurrent_backups is at least 1 */
+	Assert(max_concurrent_backups > 0);
+
+	size = offsetof(BackupCtlData, backups);
+	size = add_size(size, mul_size(max_concurrent_backups, sizeof(BackupInProgress)));
+	ShmemRequestStruct(.name = "Backup Ctl",
+		.size = size,
+		.ptr = (void **)&BackupCtl);
+}
+
+/*
+ * Initialize shared memory for the backup control structure.
+ *
+ * No cleanup is needed on shmem_exit.
+ */
+static void BackupCtlShmemInit(void *arg)
+{
+	int i;
+
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		BackupCtl->backups[i].startpoint = InvalidXLogRecPtr;
+	}
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+}
+
+/*
+ * Register this backup's startpoint.
+ *
+ * We update the oldest startpoint across all in-progress backups here.
+ *
+ */
+
+void
+RegisterBackupStartpoint(XLogRecPtr startpoint)
+{
+	int i;
+
+	Assert(BackupCtl != NULL);
+	Assert(MyBackupInProgress == NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+	for (i = 0; i < max_concurrent_backups; i++)
+	{
+		/* Find the first unused entry */
+		if (BackupCtl->backups[i].startpoint == InvalidXLogRecPtr)
+		{
+			BackupCtl->backups[i].startpoint = startpoint;
+			MyBackupInProgress = &BackupCtl->backups[i];
+			/* Update the oldest startpoint if necessary */
+			if (!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+				startpoint < BackupCtl->oldestStartpoint)
+					BackupCtl->oldestStartpoint = startpoint;
+			break;
+		}
+	}
+	LWLockRelease(BackupControlLock);
+
+	/* If the array is full, bail out */
+	if (i == max_concurrent_backups)
+		ereport(ERROR,
+				(errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+				 errmsg("maximum number of concurrent backups reached"),
+				 errhint("Wait for another backup to finish, or increase max_concurrent_backups.")));
+}
+
+/*
+ * Unregister this backup.
+ *
+ * We also recalculate the oldest startpoint across all remaining
+ * in-progress backups.
+ *
+ */
+
+void
+UnregisterBackupStartpoint(void)
+{
+	XLogRecPtr candidate_startpoint;
+
+	if (MyBackupInProgress == NULL)
+		return;
+
+	Assert(BackupCtl != NULL);
+
+	LWLockAcquire(BackupControlLock, LW_EXCLUSIVE);
+
+	MyBackupInProgress->startpoint = InvalidXLogRecPtr;
+
+	/* Invalidate the oldest startpoint */
+	BackupCtl->oldestStartpoint = InvalidXLogRecPtr;
+	/* Scan the array to find the new oldest startpoint */
+	for (int i = 0; i < max_concurrent_backups; i++)
+	{
+		candidate_startpoint = BackupCtl->backups[i].startpoint;
+		if (XLogRecPtrIsValid(candidate_startpoint) &&
+			(!XLogRecPtrIsValid(BackupCtl->oldestStartpoint) ||
+			 candidate_startpoint < BackupCtl->oldestStartpoint))
+			BackupCtl->oldestStartpoint  = candidate_startpoint;
+	}
+
+	LWLockRelease(BackupControlLock);
+
+	MyBackupInProgress = NULL;
+}
+
+/*
+ * Return the precomputed minimum startpoint across all in-progress backups
+ * to use when determining what WAL segments to keep.
+ */
+
+XLogRecPtr
+GetOldestBackupStartLSN(void)
+{
+	XLogRecPtr	retval;
+
+	LWLockAcquire(BackupControlLock, LW_SHARED);
+	retval = BackupCtl->oldestStartpoint;
+	LWLockRelease(BackupControlLock);
+
+	return retval;
+}
 
 /*
  * Build contents for backup_label or backup history file.
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index e3c04ecd810..8bdbb355b7f 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -18,6 +18,7 @@
 
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "access/xlogrecovery.h"
 #include "backup/backup_manifest.h"
 #include "backup/basebackup.h"
 #include "backup/basebackup_incremental.h"
@@ -244,6 +245,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 {
 	bbsink_state state;
 	XLogRecPtr	endptr;
+	XLogRecPtr	reserve_ptr;
 	TimeLineID	endtli;
 	backup_manifest_info manifest;
 	BackupState *backup_state;
@@ -264,6 +266,13 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 
 	backup_started_in_recovery = RecoveryInProgress();
 
+	/* Reserve the XLog before starting the backup */
+	if (backup_started_in_recovery)
+		reserve_ptr = GetXLogReplayRecPtr(NULL);
+	else
+		reserve_ptr = GetXLogInsertRecPtr();
+	RegisterBackupStartpoint(reserve_ptr);
+
 	InitializeBackupManifest(&manifest, opt->manifest,
 							 opt->manifest_checksum_type);
 
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 0a70cffa081..bb01fcf7f1a 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -371,6 +371,7 @@ WaitLSN	"Waiting to read or update shared Wait-for-LSN state."
 LogicalDecodingControl	"Waiting to read or update logical decoding status information."
 DataChecksumsWorker	"Waiting for data checksums worker."
 AioWorkerControl	"Waiting to update AIO worker information."
+BackupControl	"Waiting to update basebackup state."
 
 #
 # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3c5e16ad1e7..20dbe3946ec 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -1983,6 +1983,14 @@
   max => 'MAX_BACKENDS',
 },
 
+{ name => 'max_concurrent_backups', type => 'int', context => 'PGC_POSTMASTER', group => 'REPLICATION_SENDING',
+  short_desc => 'Sets the maximum number of simultaneously running basebackups.',
+  variable => 'max_concurrent_backups',
+  boot_val => '10',
+  min => '1',
+  max => 'MAX_BACKENDS',
+},
+
 { name => 'max_connections', type => 'int', context => 'PGC_POSTMASTER', group => 'CONN_AUTH_SETTINGS',
   short_desc => 'Sets the maximum number of concurrent connections.',
   variable => 'MaxConnections',
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e759f06b50f..186a2774ef4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -275,6 +275,8 @@
 #commit_delay = 0                       # range 0-100000, in microseconds
 #commit_siblings = 5                    # range 0-1000
 
+#max_concurrent_backups = 10            # range 1-MAX_BACKENDS  
+
 # - Checkpoints -
 
 #checkpoint_timeout = 5min              # range 30s-1d
diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h
index 2cc2f85d9f0..5d997cbfcd7 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -37,6 +37,46 @@ typedef struct BackupState
 	pg_time_t	stoptime;		/* backup stop time */
 } BackupState;
 
+/*
+ * Shared memory state of a backup in progress. Here we keep track of its
+ * start LSN to ensure checkpoints don't recycle or remove the corresponding
+ * WAL segments until we're done.
+ *
+ * Using a struct instead of a bare XLogRecPtr to allow future extensions.
+ */
+typedef struct BackupInProgress {
+	/*
+	 * Each backup triggers its own checkpoint, so their startpoints are
+	 * guaranteed to be unique, and we can use InvalidXLogRecPtr to indicate
+	 * an available entry
+	 */
+	XLogRecPtr	startpoint;
+} BackupInProgress;
+
+/* Shared memory structure for all in-progress backups
+ *
+ * It is protected by the LWLock BackupControlLock; exclusive
+ * for writers, shared for readers.
+ */
+typedef struct BackupCtlData {
+	/* Minimum startpoint of all in-progress backups */
+	XLogRecPtr			oldestStartpoint;
+	BackupInProgress 	backups[FLEXIBLE_ARRAY_MEMBER];
+} BackupCtlData;
+
+/*
+ * Pointer to shared memory
+ */
+extern PGDLLIMPORT BackupCtlData *BackupCtl;
+extern PGDLLIMPORT BackupInProgress *MyBackupInProgress;
+
+/* GUCs */
+extern PGDLLIMPORT int max_concurrent_backups;
+
+extern void RegisterBackupStartpoint(XLogRecPtr startpoint);
+extern void UnregisterBackupStartpoint(void);
+extern XLogRecPtr GetOldestBackupStartLSN(void);
+
 extern char *build_backup_content(BackupState *state,
 								  bool ishistoryfile);
 
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
index d7eb648bd27..80dfc5775a1 100644
--- a/src/include/storage/lwlocklist.h
+++ b/src/include/storage/lwlocklist.h
@@ -89,6 +89,7 @@ PG_LWLOCK(54, WaitLSN)
 PG_LWLOCK(55, LogicalDecodingControl)
 PG_LWLOCK(56, DataChecksumsWorker)
 PG_LWLOCK(57, AioWorkerControl)
+PG_LWLOCK(58, BackupControl)
 
 /*
  * There also exist several built-in LWLock tranches.  As with the predefined
diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h
index 9ad619080be..d8d1c469226 100644
--- a/src/include/storage/subsystemlist.h
+++ b/src/include/storage/subsystemlist.h
@@ -85,6 +85,7 @@ PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks)
 PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks)
+PG_SHMEM_SUBSYSTEM(BackupCtlShmemCallbacks)
 
 /* AIO subsystem. This delegates to the method-specific callbacks */
 PG_SHMEM_SUBSYSTEM(AioShmemCallbacks)
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 72113c5ac6e..db045f9cc1f 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -65,6 +65,7 @@ tests += {
       't/054_unlogged_sequence_promotion.pl',
       't/055_cascade_reconnect.pl',
       't/056_standby_snapshot_export.pl',
+      't/057_basebackup_slot_race.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/057_basebackup_slot_race.pl b/src/test/recovery/t/057_basebackup_slot_race.pl
new file mode 100644
index 00000000000..2e83f825cae
--- /dev/null
+++ b/src/test/recovery/t/057_basebackup_slot_race.pl
@@ -0,0 +1,135 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify that a base backup's startpoint survives WAL recycling triggered by
+# a concurrent checkpoint, even before pg_basebackup has created its own
+# replication slot.
+#
+# The injection point stops BASE_BACKUP after choosing its startpoint but
+# before sending it to the client.  The test recycles WAL up to and including
+# the startpoint's segment while BASE_BACKUP is paused there, then lets
+# pg_basebackup create its slot and start streaming.  The test passes when
+# the startpoint's WAL segment is still on disk and pg_basebackup succeeds,
+# proving that do_pg_backup_start() protects the segment before the slot
+# exists.
+
+use strict;
+use warnings FATAL => 'all';
+use File::Path qw(rmtree);
+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';
+}
+
+# Small WAL segments make recycling cheap.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 1, extra => [ '--wal-segsize', '1' ]);
+$node->append_conf(
+	'postgresql.conf', q[
+wal_keep_size = 0
+min_wal_size = 2MB
+max_wal_size = 4MB
+checkpoint_timeout = 1h
+]);
+$node->start;
+
+# injection_points may not be installed under installcheck.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Stop BASE_BACKUP before it sends the selected startpoint to the client.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('basebackup-before-send-startpoint', 'wait');"
+);
+
+my $backupdir = $node->backup_dir . '/basebackup_race';
+my ($bb_stdout, $bb_stderr) = ('', '');
+my $bb_timeout =
+  IPC::Run::timeout(3 * $PostgreSQL::Test::Utils::timeout_default);
+my $bb = IPC::Run::start(
+	[
+		'pg_basebackup',
+		'--pgdata' => $backupdir,
+		'--wal-method' => 'stream',
+		'--slot' => 'basebackup_race',
+		'--create-slot',
+		'--checkpoint' => 'fast',
+		'--no-sync',
+		'-d' => $node->connstr('postgres')
+	],
+	'>' => \$bb_stdout,
+	'2>' => \$bb_stderr,
+	$bb_timeout);
+
+$node->wait_for_event('walsender', 'basebackup-before-send-startpoint');
+
+# The client cannot have created its slot yet, since it hasn't received the
+# startpoint; the backup's startpoint is instead protected by the shared
+# in-progress-backup registry at this point.
+is( $node->safe_psql(
+		'postgres', 'SELECT count(*) FROM pg_replication_slots;'),
+	'0',
+	'no replication slot exists yet while startpoint is held only by the '
+	  . 'backup registry');
+
+# do_pg_backup_start() used the current checkpoint's REDO pointer.
+my $startpoint_wal = $node->safe_psql('postgres',
+	'SELECT pg_walfile_name(redo_lsn) FROM pg_control_checkpoint();');
+note "backup startpoint is in WAL segment $startpoint_wal";
+
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint exists while waiting');
+
+# Force enough WAL activity and a checkpoint to make the server want to
+# recycle the startpoint's segment.  The backup registry should keep it
+# around anyway, even though no replication slot protects it yet.
+$node->advance_wal(10);
+$node->safe_psql('postgres', 'CHECKPOINT;');
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+	),
+	'1',
+	'WAL segment containing the backup startpoint survives a concurrent '
+	  . 'checkpoint'
+);
+
+# Let pg_basebackup create the slot and request WAL starting at the
+# (still-present) startpoint.
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('basebackup-before-send-startpoint');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('basebackup-before-send-startpoint');");
+
+$bb->finish;
+note "pg_basebackup stderr:\n$bb_stderr";
+
+is($bb->result(0), 0, 'pg_basebackup succeeded despite concurrent WAL recycling')
+  or diag "pg_basebackup stdout: $bb_stdout\npg_basebackup stderr: $bb_stderr";
+
+# The slot requested via --create-slot should now exist.
+is( $node->safe_psql(
+		'postgres',
+		"SELECT count(*) FROM pg_replication_slots WHERE slot_name = "
+		  . "'basebackup_race';"),
+	'1',
+	'replication slot was created once pg_basebackup received the startpoint'
+);
+
+rmtree($backupdir);
+$node->safe_psql('postgres',
+	"SELECT pg_drop_replication_slot(slot_name) FROM pg_replication_slots "
+	  . "WHERE slot_name = 'basebackup_race';"
+);
+
+done_testing();
-- 
2.50.1 (Apple Git-155)



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-10 08:01  Andrey Borodin <x4mmm@yandex-team.ru>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Andrey Borodin @ 2026-09-10 08:01 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hi Nick,

On 7 Sep 2026, Nick Ivanov wrote:
> reserve the WAL earlier in the process

This addresses the primary-side window.  I should clarify my earlier
suggestion for standbys, though: reserving the replay LSN and later
lowering it is insufficient without an interlock with WAL removal.  A
restartpoint can remove the older start segment between selecting it and
registering it.  Updating the entry in place would avoid the
unregister/register gap, but would not by itself close that earlier
window.

The reservation also needs cleanup from the moment it is registered.
For example, the "backup label too long" error in do_pg_backup_start()
occurs before its cleanup block and leaves the new array entry behind.

For -X fetch, retention ends too early: do_pg_backup_stop() unregisters
the startpoint before perform_base_backup() copies the WAL.  I paused at
an injection point just before the opt->includewal block, generated WAL,
and ran CHECKPOINT.  The start segment disappeared and v2 failed with
the missing-WAL error.  Retention and its error cleanup need to cover
the WAL copy too.

KeepLogSeg() also applies max_slot_wal_keep_size to the backup
reservation.  That allows a checkpoint to discard required WAL even while
the reservation exists.  Should that limit apply to backups at all?
Unlike slots, these reservations have no invalidation mechanism.

> and the introduction of a new GUC should therefore be acceptable.

Even for HEAD, max_wal_senders already bounds concurrent BASE_BACKUP
commands, including -X fetch: they all run in walsender processes.
Could we size the array from that instead?

I think the client-side fix is worth backpatching, even if this broader
work stays on HEAD.

One build note: my build warned about the missing declaration of
BackupCtlShmemCallbacks.  xlogbackup.c needs storage/subsystems.h for it.

Thank you!


Best regards, Andrey Borodin.







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-11 16:25  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Andrey Borodin <x4mmm@yandex-team.ru>
  0 siblings, 1 reply; 16+ messages in thread

From: Nick Ivanov @ 2026-09-11 16:25 UTC (permalink / raw)
  To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

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.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, &param->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.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, &param->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)



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-15 18:29  Andrey Borodin <x4mmm@yandex-team.ru>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 2 replies; 16+ messages in thread

From: Andrey Borodin @ 2026-09-15 18:29 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hi Nick,

On 11 Sep 2026, Nick Ivanov wrote:
> I agree, and I'm attaching a separate patch for that.

Thanks!  This looks like the right scope for a backpatch.

I adapted your v2 test for the client-side fix, checking that the slot
already reserves WAL before the server sends the startpoint.  It covers
both --create-slot and the default temporary slot, and requires the
backup to succeed after the concurrent checkpoint.  Without the fix,
both cases fail with the expected missing-WAL error.

Small wording detail. Another checkpoint is enough to trigger the race.
It need not come from another basebackup.  I adjusted and wrapped the
commit message accordingly.  Apart from wrapping a comment, the client
code is unchanged.

WDYT?


Best regards, Andrey Borodin.

Attachments:

  [application/octet-stream] v3-0001-Test-WAL-retention-before-pg_basebackup-starts-st.patch (6.2K, ../../2A5B29E9-658D-4AD7-8879-C6169D1327E3@yandex-team.ru/2-v3-0001-Test-WAL-retention-before-pg_basebackup-starts-st.patch)
  download | inline diff:
From 89b0aa3333e5616972ec802bd2ef4f85b91dd6f2 Mon Sep 17 00:00:00 2001
From: Nick Ivanov <nick.ivanov@enterprisedb.com>
Date: Tue, 15 Sep 2026 21:11:05 +0500
Subject: [PATCH v3 1/2] Test WAL retention before pg_basebackup starts
 streaming

A concurrent checkpoint can remove the backup's starting WAL segment
before pg_basebackup creates its replication slot.  Stop the backup
before it sends the startpoint and force WAL recycling to exercise this
window with both a permanent and a temporary slot.

Adapt the test from the server-side proposal to check that the slot
already reserves WAL at this point and that the backup succeeds.

Discussion: https://www.postgresql.org/message-id/0cab2102-4786-416a-ae50-b606a495d01b@enterprisedb.com
---
 src/backend/backup/basebackup.c               |   3 +
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/057_basebackup_slot_race.pl    | 129 ++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 src/test/recovery/t/057_basebackup_slot_race.pl

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index e3c04ecd810..e6733058136 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -324,6 +324,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 			state.bytes_total_is_valid = true;
 		}
 
+		/* Allow tests to wait before the client receives the startpoint. */
+		INJECTION_POINT("basebackup-before-send-startpoint", NULL);
+
 		/* notify basebackup sink about start of backup */
 		bbsink_begin_backup(sink, &state, SINK_BUFFER_LENGTH);
 
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 72113c5ac6e..db045f9cc1f 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -65,6 +65,7 @@ tests += {
       't/054_unlogged_sequence_promotion.pl',
       't/055_cascade_reconnect.pl',
       't/056_standby_snapshot_export.pl',
+      't/057_basebackup_slot_race.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/057_basebackup_slot_race.pl b/src/test/recovery/t/057_basebackup_slot_race.pl
new file mode 100644
index 00000000000..3145a834da4
--- /dev/null
+++ b/src/test/recovery/t/057_basebackup_slot_race.pl
@@ -0,0 +1,129 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify that pg_basebackup reserves WAL before requesting its startpoint.
+#
+# Hold BASE_BACKUP before sending the startpoint, generate WAL, and checkpoint.
+# The early-created slot must retain the start segment until streaming starts.
+
+use strict;
+use warnings FATAL => 'all';
+use File::Path qw(rmtree);
+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';
+}
+
+# Small WAL segments make recycling cheap.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init(allows_streaming => 1, extra => [ '--wal-segsize', '1' ]);
+$node->append_conf(
+	'postgresql.conf', q[
+wal_keep_size = 0
+min_wal_size = 2MB
+max_wal_size = 4MB
+checkpoint_timeout = 1h
+]);
+$node->start;
+
+# injection_points may not be installed under installcheck.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+for my $mode ('permanent', 'temporary')
+{
+	note "Testing $mode replication slot";
+
+	# Stop BASE_BACKUP before it sends the selected startpoint to the client.
+	$node->safe_psql('postgres',
+		"SELECT injection_points_attach('basebackup-before-send-startpoint', 'wait');"
+	);
+
+	my $backupdir = $node->backup_dir . '/basebackup_race_' . $mode;
+	my ($bb_stdout, $bb_stderr) = ('', '');
+	my $bb_timeout =
+	  IPC::Run::timeout(3 * $PostgreSQL::Test::Utils::timeout_default);
+	my $bb = IPC::Run::start(
+		[
+			'pg_basebackup',
+			'--pgdata' => $backupdir,
+			'--wal-method' => 'stream',
+			(   $mode eq 'permanent'
+				? ('--slot' => 'basebackup_race', '--create-slot')
+				: ()),
+			'--checkpoint' => 'fast',
+			'--no-sync',
+			'-d' => $node->connstr('postgres')
+		],
+		'>' => \$bb_stdout,
+		'2>' => \$bb_stderr,
+		$bb_timeout);
+
+	$node->wait_for_event('walsender', 'basebackup-before-send-startpoint');
+
+	# The slot must already reserve WAL before the client receives the startpoint.
+	is( $node->safe_psql(
+			'postgres',
+			'SELECT count(*) FROM pg_replication_slots '
+			  . 'WHERE restart_lsn IS NOT NULL;'),
+		'1',
+		'replication slot reserves WAL before receiving the startpoint');
+
+	# do_pg_backup_start() used the current checkpoint's REDO pointer.
+	my $startpoint_wal = $node->safe_psql('postgres',
+		'SELECT pg_walfile_name(redo_lsn) FROM pg_control_checkpoint();');
+	note "backup startpoint is in WAL segment $startpoint_wal";
+
+	is( $node->safe_psql(
+			'postgres',
+			"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+		),
+		'1',
+		'WAL segment containing the backup startpoint exists while waiting');
+
+	# Force enough WAL activity and a checkpoint to recycle the start segment
+	# unless the replication slot protects it.
+	$node->advance_wal(10);
+	$node->safe_psql('postgres', 'CHECKPOINT;');
+	is( $node->safe_psql(
+			'postgres',
+			"SELECT count(*) FROM pg_ls_waldir() WHERE name = '$startpoint_wal';"
+		),
+		'1',
+		'WAL segment containing the backup startpoint survives a concurrent '
+		  . 'checkpoint');
+
+	# Let pg_basebackup request WAL from the retained startpoint.
+	$node->safe_psql('postgres',
+		"SELECT injection_points_wakeup('basebackup-before-send-startpoint');"
+	);
+	$node->safe_psql('postgres',
+		"SELECT injection_points_detach('basebackup-before-send-startpoint');"
+	);
+
+	$bb->finish;
+	note "pg_basebackup stderr:\n$bb_stderr";
+
+	is($bb->result(0), 0,
+		'pg_basebackup succeeded despite concurrent WAL recycling')
+	  or diag
+	  "pg_basebackup stdout: $bb_stdout\npg_basebackup stderr: $bb_stderr";
+
+	rmtree($backupdir);
+	if ($mode eq 'permanent')
+	{
+		$node->safe_psql('postgres',
+			"SELECT pg_drop_replication_slot('basebackup_race');");
+	}
+	ok( $node->poll_query_until(
+			'postgres', 'SELECT count(*) = 0 FROM pg_replication_slots;'),
+		'replication slot cleaned up');
+}
+
+done_testing();
-- 
That's all, folks. May the source be with you.

=

  [application/octet-stream] v3-0002-Create-pg_basebackup-s-replication-slot-before-st.patch (5.2K, ../../2A5B29E9-658D-4AD7-8879-C6169D1327E3@yandex-team.ru/3-v3-0002-Create-pg_basebackup-s-replication-slot-before-st.patch)
  download | inline diff:
From 525edd0e35111b19b97d3fd935042be138a328d8 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 v3 2/2] Create pg_basebackup's replication slot before
 starting the backup

A checkpoint can remove the backup's starting WAL segment before
pg_basebackup creates its replication slot.  Creating the slot after
receiving the startpoint is too late to protect that segment.

Create the slot and reserve WAL before sending BASE_BACKUP, keeping the
same connection for the WAL streamer.  This covers both requested
permanent slots and automatically created temporary slots.

Backpatch-through: 15
Discussion: https://www.postgresql.org/message-id/985de9f0-cbb6-4235-a6cd-32242f74e1f3@enterprisedb.com
---
 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..00db4b6784a 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, &param->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);
 	}
-- 
That's all, folks. May the source be with you.

=

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-18 03:02  Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>
  parent: Andrey Borodin <x4mmm@yandex-team.ru>
  1 sibling, 1 reply; 16+ messages in thread

From: Shashishekar Hullahally Anantharamu @ 2026-09-18 03:02 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org; +Cc: nick ivanov <nick@thebeaches.online>

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Hi Andrey and Nick,

I reviewed the v3 two-patch series against PostgreSQL commit 94670ba6d56.

Both patches applied cleanly, and git diff --check reported no errors. I configured the build with assertions, debug support, TAP tests, and injection points enabled. The build completed successfully without warnings or errors.

The implementation moves creation of the WAL-streaming connection and replication slot before the BASE_BACKUP request. It then passes the same connection to StartLogStreamer(). This closes the interval in which the backup startpoint could previously become unprotected before the requested slot was created.

I also reviewed the new 057_basebackup_slot_race.pl test. It deterministically pauses BASE_BACKUP before the startpoint is returned, forces WAL generation and a checkpoint, and verifies that the startpoint segment remains available. The test covers both permanent and temporary replication slots and verifies successful backup completion and slot cleanup.

Test results:

057_basebackup_slot_race.pl: PASS, 10 tests
Complete src/bin/pg_basebackup test suite: PASS, 5 files and 351 tests
Full make check: PASS

The first component-suite and full-check attempts encountered macOS temporary-install Mach-O paths referring to /usr/local/pgsql/lib/libpq.5.dylib. After correcting those paths only in the disposable temporary installation, the affected tests and complete suites passed. This was a local build-environment issue and did not require any source changes.

I did not find any functional or test-coverage issues with the v3 series. The patch appears ready for committer review.

Regards,
Shashishekar Hullahally Anantharamu

The new status of this patch is: Ready for Committer


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-19 13:48  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Andrey Borodin <x4mmm@yandex-team.ru>
  1 sibling, 0 replies; 16+ messages in thread

From: Nick Ivanov @ 2026-09-19 13:48 UTC (permalink / raw)
  To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>

Hello Andrey,

On 15/09/2026 19:29, Andrey Borodin wrote:
> Hi Nick,
>
> Thanks!  This looks like the right scope for a backpatch.
>
> I adapted your v2 test for the client-side fix, checking that the slot
> already reserves WAL before the server sends the startpoint.  It covers
> both --create-slot and the default temporary slot, and requires the
> backup to succeed after the concurrent checkpoint.  Without the fix,
> both cases fail with the expected missing-WAL error.

Thank you for updating the test, much appreciated. I should have done 
that myself, to be honest.


> Small wording detail. Another checkpoint is enough to trigger the race.
> It need not come from another basebackup.  I adjusted and wrapped the
> commit message accordingly.  Apart from wrapping a comment, the client
> code is unchanged.
>
> WDYT?


The changes make good sense, thanks for that too.

I will now proceed to validate the patch against older versions. One 
question in that regard: the TAP test carries the number 57 in the 
recovery suite in the master branch. Earlier stable versions likely have 
fewer tests, and if we add the new test there with #57 there will be a 
gap in the sequence. What is the accepted practice in such cases: 
renumber the newly added test in earlier versions to avoid the gap, or 
keep the number consistent with HEAD?

Cheers


Nick






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-19 13:50  Nick Ivanov <nick.ivanov@enterprisedb.com>
  parent: Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Nick Ivanov @ 2026-09-19 13:50 UTC (permalink / raw)
  To: Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>; pgsql-hackers@lists.postgresql.org; +Cc: nick ivanov <nick@thebeaches.online>

Hello Shashishekar,

Thank you very much for your review and tests, much appreciated.

Nick

On 18/09/2026 04:02, Shashishekar Hullahally Anantharamu wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  not tested
> Implements feature:       tested, passed
> Spec compliant:           not tested
> Documentation:            not tested
>
> Hi Andrey and Nick,
>
> I reviewed the v3 two-patch series against PostgreSQL commit 94670ba6d56.
>
> Both patches applied cleanly, and git diff --check reported no errors. I configured the build with assertions, debug support, TAP tests, and injection points enabled. The build completed successfully without warnings or errors.
>
> The implementation moves creation of the WAL-streaming connection and replication slot before the BASE_BACKUP request. It then passes the same connection to StartLogStreamer(). This closes the interval in which the backup startpoint could previously become unprotected before the requested slot was created.
>
> I also reviewed the new 057_basebackup_slot_race.pl test. It deterministically pauses BASE_BACKUP before the startpoint is returned, forces WAL generation and a checkpoint, and verifies that the startpoint segment remains available. The test covers both permanent and temporary replication slots and verifies successful backup completion and slot cleanup.
>
> Test results:
>
> 057_basebackup_slot_race.pl: PASS, 10 tests
> Complete src/bin/pg_basebackup test suite: PASS, 5 files and 351 tests
> Full make check: PASS
>
> The first component-suite and full-check attempts encountered macOS temporary-install Mach-O paths referring to /usr/local/pgsql/lib/libpq.5.dylib. After correcting those paths only in the disposable temporary installation, the affected tests and complete suites passed. This was a local build-environment issue and did not require any source changes.
>
> I did not find any functional or test-coverage issues with the v3 series. The patch appears ready for committer review.
>
> Regards,
> Shashishekar Hullahally Anantharamu
>
> The new status of this patch is: Ready for Committer






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Possible race condition in pg_basebackup
@ 2026-09-23 15:07  Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>
  parent: Nick Ivanov <nick.ivanov@enterprisedb.com>
  0 siblings, 0 replies; 16+ messages in thread

From: Shashishekar Hullahally Anantharamu @ 2026-09-23 15:07 UTC (permalink / raw)
  To: Nick Ivanov <nick.ivanov@enterprisedb.com>; +Cc: pgsql-hackers@lists.postgresql.org, nick ivanov <nick@thebeaches.online>

Hi Nick,

You are welcome!, Glad that my tests and reviews were helpful.

Regards
Shashi A

On Sat, Sep 19, 2026 at 8:50 AM Nick Ivanov <nick.ivanov@enterprisedb.com>
wrote:

> Hello Shashishekar,
>
> Thank you very much for your review and tests, much appreciated.
>
> Nick
>
> On 18/09/2026 04:02, Shashishekar Hullahally Anantharamu wrote:
> > The following review has been posted through the commitfest application:
> > make installcheck-world:  not tested
> > Implements feature:       tested, passed
> > Spec compliant:           not tested
> > Documentation:            not tested
> >
> > Hi Andrey and Nick,
> >
> > I reviewed the v3 two-patch series against PostgreSQL commit 94670ba6d56.
> >
> > Both patches applied cleanly, and git diff --check reported no errors. I
> configured the build with assertions, debug support, TAP tests, and
> injection points enabled. The build completed successfully without warnings
> or errors.
> >
> > The implementation moves creation of the WAL-streaming connection and
> replication slot before the BASE_BACKUP request. It then passes the same
> connection to StartLogStreamer(). This closes the interval in which the
> backup startpoint could previously become unprotected before the requested
> slot was created.
> >
> > I also reviewed the new 057_basebackup_slot_race.pl test. It
> deterministically pauses BASE_BACKUP before the startpoint is returned,
> forces WAL generation and a checkpoint, and verifies that the startpoint
> segment remains available. The test covers both permanent and temporary
> replication slots and verifies successful backup completion and slot
> cleanup.
> >
> > Test results:
> >
> > 057_basebackup_slot_race.pl: PASS, 10 tests
> > Complete src/bin/pg_basebackup test suite: PASS, 5 files and 351 tests
> > Full make check: PASS
> >
> > The first component-suite and full-check attempts encountered macOS
> temporary-install Mach-O paths referring to
> /usr/local/pgsql/lib/libpq.5.dylib. After correcting those paths only in
> the disposable temporary installation, the affected tests and complete
> suites passed. This was a local build-environment issue and did not require
> any source changes.
> >
> > I did not find any functional or test-coverage issues with the v3
> series. The patch appears ready for committer review.
> >
> > Regards,
> > Shashishekar Hullahally Anantharamu
> >
> > The new status of this patch is: Ready for Committer
>

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

end of thread, other threads:[~2026-09-23 15:07 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-21 10:30 Possible race condition in pg_basebackup Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-08-21 11:28 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-21 16:35 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-08-24 07:56   ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-08-25 06:32   ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-28 18:05     ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-08-29 14:18       ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-31 12:32         ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-09-07 14:19           ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-09-10 08:01             ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-09-11 16:25               ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-09-15 18:29                 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-09-18 03:02                   ` Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>
2026-09-19 13:50                     ` Nick Ivanov <nick.ivanov@enterprisedb.com>
2026-09-23 15:07                       ` Shashishekar Hullahally Anantharamu <shashi.h.ananth@gmail.com>
2026-09-19 13:48                   ` Nick Ivanov <nick.ivanov@enterprisedb.com>

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