| From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | RE: Fix race in ReplicationSlotRelease for ephemeral slots |
| Date: | 2026-06-16 10:32:14 |
| Message-ID: | TY4PR01MB177185B82B7BD0A20028E4CED94E52@TY4PR01MB17718.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tuesday, June 16, 2026 5:36 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jun 16, 2026 at 2:24 PM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> >
> > I have one minor comment for the 0001 patch.
> >
> > + NameData slot_name = {0};
> > ...
> > SpinLockAcquire(&local_slot->mutex);
> > synced_slot = local_slot->in_use &&
> > local_slot->data.synced;
> > + if (synced_slot)
> > + slot_name = local_slot->data.name;
> > SpinLockRelease(&local_slot->mutex);
> >
> > We can defer assigning slot_name until after we pass the existing
> > (synced_slot) check. Since it's a synced slot, no other process can
> > change it at that point, and we can also skip initializing slot_name.
> > (Please refer to the attached patch for suggested changes)
> >
>
> + if (dropped)
> + ereport(LOG,
> + errmsg("dropped replication slot \"%s\" of database with OID %u",
> + NameStr(slot_name),
> + slot_database));
>
> Can we avoid the if (dropped) check by placing this LOG message immediately
> after dropping the slot under synced slot check?
I think we can do that. I'm attaching the new patches for all supported
branches, incorporating both my and Amit's comments. I hope this helps move the
fix forward.
I also confirmed that the fix works on all supported branches.
Best Regards,
Hou zj
| Attachment | Content-Type | Size |
|---|---|---|
| v2_PG17-0001-Avoid-stale-slot-access-after-dropping-obsol.patch | application/octet-stream | 2.7 KB |
| v2-0001-Avoid-stale-slot-access-after-dropping-obsolete-s.patch | application/octet-stream | 2.9 KB |
| v2_PG18-0001-Avoid-stale-slot-access-after-dropping-obsol.patch | application/octet-stream | 2.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dilip Kumar | 2026-06-16 10:41:55 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Ewan Young | 2026-06-16 10:27:19 | Re: GRAPH_TABLE: aggregates/window/set-returning functions in COLUMNS crash the backend |