Re: Timing issue in pg_basebackup

Hi Jesper,

Thank you for the patch. I tested v3-0001 (CF entry 7613) and here are my
results.

Setup: master at c1694b7 (11 Nov 2026), configured with --enable-cassert,
--enable-tap-tests and --enable-injection-points, on Linux (WSL Ubuntu). The
patch applied cleanly with git apply.

1. The bug on unpatched master

I held the walsender at the existing injection point
replication-slot-create-begin (so BASE_BACKUP has returned, but the slot
does not exist yet). Then I switched WAL four times, ran CHECKPOINT, and
released the walsender. The server was running with wal_keep_size=0.

pg_basebackup: error: unexpected termination of replication stream: ERROR:
requested WAL segment 000000010000000000000002 has already been removed
pg_basebackup: error: background process terminated unexpectedly

pg_basebackup exited with 1. So the race you describe is real, and it can be
triggered on demand.

2. With v3 applied

The same script gives pg_basebackup exit 0, no error in stderr or in the
server log, "backup successfully verified" from pg_verifybackup, and no
leftover slot.

3. Tests

"make -C src/bin/pg_basebackup check" with the patch: all 7 files pass (372
tests), including the new 013_wal_slot_timing.pl and the new cases in
010_pg_basebackup.pl. I ran 013 alone with prove -v to confirm it was not
skipped: "ok 1 - pg_basebackup succeeds with WAL activity during slot
creation", "ok 2 - no WAL needed by the backup was removed", "ok 3 - backup
with streamed WAL verifies", Files=1, Tests=3, Result: PASS. As a negative
control, I kept the new tests but put back the original pg_basebackup.c.
Then 013 fails all 3 subtests with the same "has already been removed"
error, so the test does detect the bug.

4. A note on lingering slots and signals

You and Kirill discussed the lingering slot in the thread. I tried to see
what happens on a signal. With v3 the slot is created before the checkpoint,
so the window in which an interrupted pg_basebackup can leave the slot
behind now also covers the checkpoint wait. With --checkpoint=spread that
can be long.

To compare fairly, I did the test after the slot exists on both builds (on
master the slot is only created after BASE_BACKUP returns, so a SIGTERM
during the checkpoint finds no slot there). I ran pg_basebackup -Xstream
--create-slot -S s1 --checkpoint=fast --max-rate=1M on a 216 MB database,
polled pg_replication_slots until s1 appeared, and then sent SIGTERM while
the backup was still running:

- master: s1 was still there after SIGTERM (and still there 11 s later).
- with v3: s1 was still there after SIGTERM (and still there 11 s later).

So master also leaves the slot behind on SIGTERM once the slot exists. I
think this is expected, since an atexit() handler probably does not run when
the process is killed by a signal, but I did not test that part directly. v3
does not make this worse for a signal after the slot is created. It only
starts the window earlier, so it is longer in time (the checkpoint wait).
The slot is not temporary here because I used --create-slot. The walsender
state differed between runs: in the first run s1 showed as inactive right
after the kill with v3 (active on master), and in a second run s1 was still
active on both builds 11 s after the kill.

Your atexit() handler works for the error cases covered by the TAP tests. If
you want to cover signals too, maybe a signal handler could drop the slot,
or the docs for --create-slot / --checkpoint could say that a slot may
remain if the process is killed. This is just a suggestion, I do not see it
as a regression of the patch. I only tested SIGTERM.

Small comment on the code: the check "success || in_log_streamer || bgchild
> 0" looks fine to me in the cases I ran, but I did not review all exit
paths in detail.

What I did NOT test:
- Other platforms (Windows, where pg_basebackup uses a thread instead of
fork, and the new atexit() and bgconn handling matter).
- Other WAL methods and formats (--wal-method=fetch/none, tar format,
compressed WAL, --format=tar with stream) beyond what the existing TAP
suite covers.
- Older server versions (the MINIMUM_VERSION_FOR_TEMP_SLOTS branch).
- SIGINT, and failures after the background WAL receiver has started.
- Meson build (I used make/autoconf only), and the docs/pgindent formatting.
- The real two-node ansible scenario from your first mail (I only reproduced
the race with an injection point), and the performance effect of holding
the slot during long spread checkpoints (only the slot visibility was
checked, above).
- Whether other tools that use the same pattern (for example pg_receivewal
--create-slot) need the same change.

The scripts and the full log are available if you want them. Thanks again
for working on this.

Best regards,
Joao Rito