Possible race condition in pg_basebackup

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]/messages/by-id/1505248760.16872.1.camel@credativ.de. It was thought that a subsequent checkpoint before the WAL
streamer connected was unlikely enough that the concern might be only
theoretical [1]/messages/by-id/CAMkU=1z_OucAFdersKxqjKsNUwFCrjvqV0=d-5o9Q17nR53s_g@mail.gmail.com. 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]https://www.postgresql.org/docs/current/app-pgbasebackup.html. 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]: /messages/by-id/1505248760.16872.1.camel@credativ.de
[1]: /messages/by-id/CAMkU=1z_OucAFdersKxqjKsNUwFCrjvqV0=d-5o9Q17nR53s_g@mail.gmail.com
[2]: https://www.postgresql.org/docs/current/app-pgbasebackup.html

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:
/messages/by-id/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com

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: /messages/by-id/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: /messages/by-id/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)

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:

/messages/by-id/5e045179-236f-4f8f-84f1-0f2566ba784c.mengjuan.cmj@alibaba-inc.com

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:
/messages/by-id/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:
/messages/by-id/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

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.

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

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.

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

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

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.

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]/messages/by-id/99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru.

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]: /messages/by-id/99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru
/messages/by-id/99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru

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.

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

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

Hello Shashishekar,

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

Nick

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:

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 Nick, Andrey,
I tested v3 on master (45fc51f, 2026-10-17), built with --enable-cassert --enable-debug --enable-tap-tests --enable-injection-points on Linux (aarch64).

v3 no longer applies (cfbot shows "Needs rebase"): 0001 conflicts in src/test/recovery/meson.build, and 057 is now taken by 057_snapshot_commit_race.pl (058-060 exist as well). 0002 applies cleanly. For testing I renamed the test to 061_basebackup_slot_race.pl.

- No compiler warnings.
- The new test passes with 0001+0002. With 0001 only (the injection point and the test, no fix) it fails in both modes with "requested WAL segment ... has already been removed", as described upthread.
- src/bin/pg_basebackup's TAP tests pass with the patch.
- Without injection points, the reported scenario: three pg_basebackup -X stream --create-slot -c fast started together, 1 MB WAL segments, wal_keep_size = 0, max_wal_size = 32MB, and a session writing WAL continuously (pg_logical_emit_message, 1 MB each). Over 100 rounds (300 backups), 62 failed without 0002, all with "requested WAL segment ... has already been removed", and none failed with it. All 538 backups that completed pass pg_verifybackup.
One behaviour change I noticed: with --create-slot, the permanent slot is now created before BASE_BACKUP, and pg_basebackup doesn't drop it if the backup can't start. Before the patch, a backup the server refused never created the slot. To check:

initdb, summarize_wal = on, pg_basebackup a full backup, then summarize_wal = off, then
pg_basebackup -X stream --create-slot -S leftover --incremental=<full>/backup_manifest
-> could not initiate base backup: ERROR: incremental backups cannot be taken unless WAL summarization is enabled
On master no slot is left. With the patch, "leftover" remains with restart_lsn 0/02000028 and keeps holding back WAL until someone drops it. Since the slot is now created first, any other error from BASE_BACKUP should leave it behind the same way (I only tried this one). Temporary slots are not affected, since they go away with the connection.

Would it make sense to drop the slot when BASE_BACKUP fails, if this run created it? A failure later in the backup already leaves the slot behind today, so that part isn't new, but a refused start probably shouldn't leave a permanent slot behind.

I haven't tried the back branches yet.

Regards,
Sai Pranav

The new status of this patch is: Waiting on Author

Hi Nick, Andrey,

Following up on my review with the back branches (REL_15_STABLE to REL_19_STABLE, as of 2026-10-19), since the patch is marked Backpatch-through: 15.

0002 doesn't apply as is on any of them:
- in StartLogStreamer() the context line `if (!pg_parse_lsn(startpos, ...))` is `sscanf(startpos, "%X/%X", ...)` there (`"%X/%08X"` on 19), so the `param->bgconn = walconn;` hunk has to be placed by hand
- on 15, 16 and 17 CreateReplicationSlot() has no failover argument, so the new call takes 8 arguments

With only those changes it builds without warnings, src/bin/pg_basebackup's TAP tests pass, and the race (three pg_basebackup --create-slot started together, 1 MB segments, wal_keep_size = 0, a session writing WAL) gives 0 failures of 300 backups on each branch, against 150-165 of 300 without it.

0001 on 17-19 needs `#include "utils/injection_point.h"` in basebackup.c (master has it, the branches don't); on 17, INJECTION_POINT() takes one argument and Cluster.pm has no check_extension(). The free test number also differs per branch (057 on 17, 058 on 18 and 19). With that, the test passes with 0002 and fails in both modes without it on 17, 18 and 19.

The leftover slot from my previous mail isn't tied to incremental backups: with `pg_basebackup -X stream --create-slot -S leftover --manifest-checksums=bogus` the server rejects BASE_BACKUP (`unrecognized checksum algorithm: "bogus"`), and with the patch the slot stays, on every branch from 15 to 19; without it there is none.

Regards,
Pranav