| 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 mailing list <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Possible race condition in pg_basebackup |
| Date: | 2026-09-11 16:25:06 |
| Message-ID: | 985de9f0-cbb6-4235-a6cd-32242f74e1f3@enterprisedb.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0002-pg_basebackup-to-create-replication-slot-early.patch | text/plain | 4.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-09-11 16:27:57 | Re: pg_get_*_ddl() needs a redesign |
| Previous Message | Vik Fearing | 2026-09-11 16:18:41 | Re: Logical Implication |