Re: Possible race condition in pg_basebackup

From: Nick Ivanov <nick(dot)ivanov(at)enterprisedb(dot)com>
To: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Possible race condition in pg_basebackup
Date: 2026-08-28 18:05:43
Message-ID: CALP_NYQZHLp4+so=JXJwVonP8Rkz2nx4uwRZOP1DXGtY_gW8eQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

On Fri, Aug 21, 2026 at 12:28 PM Andrey Borodin <x4mmm(at)yandex-team(dot)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

Attachment Content-Type Size
v1-0001-Fix-WAL-recycle-race-in-pg_basebackup.patch application/octet-stream 18.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2026-08-28 18:12:34 Re: pg_stat_get_autovacuum_scores ignores the main table's reloptions for TOAST tables
Previous Message Diego 2026-08-28 17:48:44 Re: [Proposal] add portaddr like hostaddr