On Tue, Jun 16, 2026 at 8:46 PM Fujii Masao <[email protected]> wrote:
>
> On Fri, Jun 12, 2026 at 7:54 PM Amit Kapila <[email protected]> wrote:
> > I feel even if there is an argument to do such a refactoring, it can
> > be done separately. We can push forward with 0001 and then do more
> > discussion for 0002, if required. I can take care of 0001 unless
> > Fujii-San wishes to take care of it?
>
> Yeah, please feel free to work on 0001.
>
> Regarding 0002, since the race is very rare and non-fatal, I'm okay
> with accepting the risk rather than adding more refactoring just to
> avoid it.
>
> I'm a bit tempted to add a source comment explaining the risk and
> why we accept it, though, so other developers can understand
> the tradeoff. For example:
>
> diff --git a/src/backend/replication/logical/slotsync.c
> b/src/backend/replication/logical/slotsync.c
> index 05637344363..ca49f20e7d9 100644
> --- a/src/backend/replication/logical/slotsync.c
> +++ b/src/backend/replication/logical/slotsync.c
> @@ -560,6 +560,12 @@ drop_local_obsolete_slots(List *remote_slot_list)
>                          * the same shared memory as that of
> 'local_slot'. Thus check if
>                          * local_slot is still the synced one before
> performing the actual
>                          * drop.
> +                        *
> +                        * Because local_slot still points to a
> reusable slot-array entry,
> +                        * fields such as name or database OID could
> already be stale here.
> +                        * That could cause an incorrect cleanup
> decision for this cycle or
> +                        * briefly lock an unrelated database. We
> accept that risk because
> +                        * this race is rare and non-fatal.
>                          */
>                         SpinLockAcquire(&local_slot->mutex);
>                         synced_slot = local_slot->in_use &&
> local_slot->data.synced;

Thanks for suggesting the comment! It helps to clarify the situation
and the trade-off we made here. I tweaked it a bit and added it to the
patches prepared by Zhijie.


--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

Attachment: v3_PG17-0001-Avoid-stale-slot-access-after-dropping-obsol.patch
Description: Binary data

Attachment: v3-0001-Avoid-stale-slot-access-after-dropping-obsolete-s.patch
Description: Binary data

Attachment: v3_PG18-0001-Avoid-stale-slot-access-after-dropping-obsol.patch
Description: Binary data

Reply via email to