From: Nick Ivanov <nick.ivanov@enterprisedb.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Álvaro Herrera <alvherre@kurilemu.de>
Cc: pgsql-hackers mailing list <pgsql-hackers@lists.postgresql.org>
Subject: Re: Possible race condition in pg_basebackup
Date: Mon, 31 Aug 2026 13:32:25 +0100
Message-ID: <999caa0e-d015-42da-9b79-001087e719b8@enterprisedb.com> (raw)
In-Reply-To: <99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru>
References: <aoh9KdCo12Y07mZO@alvherre.pgsql>
<5516902D-65A5-4C61-8568-E32F111C89EF@yandex-team.ru>
<CALP_NYQZHLp4+so=JXJwVonP8Rkz2nx4uwRZOP1DXGtY_gW8eQ@mail.gmail.com>
<99DF8255-F6B8-4B10-85A2-B6984B3DD16C@yandex-team.ru>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nick.ivanov@enterprisedb.com, x4mmm@yandex-team.ru, alvherre@kurilemu.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Possible race condition in pg_basebackup
In-Reply-To: <999caa0e-d015-42da-9b79-001087e719b8@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox