On Mon, 17 Mar 2025 at 12:18, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Mar 10, 2025 at 11:46 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> >
> > I tried to reproduce the scenario described using the following steps:
> >
> > Here are the steps:
> >
> > 1. set max_replication_slots = 2
> >
> > 2. create two logical replication slot, lets say slot1 and slot2. drop
> > the first slot (slot1).
> >
> > 3. Now run pg_copy_logical_replication slot function on slot2. Lets
> > say copied slot is 'slot_cp'.
> > In function copy_replication_slot add breakpoint just after function
> > 'create_logical_replication_slot'.
> > slot_cp will be created. In array
> > ReplicationSlotCtl->replication_slots, slot_cp will come before slot2.
> >
> > 4. Now invalidate the 'slot2'.
> > Function 'InvalidateObsoleteReplicationSlots' will be called. Now it
> > will loop through ReplicationSlotCtl->replication_slots and will get
> > 'slot_cp' first. Since the slot is acquired by copy_replication_slot,
> > It will wait for the slot to be released. Once slot is released,
> > 'slot_cp' and 'slot2' will be invalidated.
> >
>
> It is also possible that InvalidateObsoleteReplicationSlots() has
> already past slot_cp location in which case it doesn't need to wait
> for the backend.
>
> I have changed the comments for the master branch patch. Do let me
> know what you think of the attached change. If you agree with this,
> then please prepare the patches for back-branches that reflect this
> change.
>
This change looks good to me. I have prepared the patches with the
suggested changes for PG16, PG17 and master.
I have also addressed the comments by Sawada-san in [1] and added a
line in the commit message for PG15 and earlier version.
[1]: https://www.postgresql.org/message-id/CAD21AoBtjj_xoH%2BpgHP_CKykj00fayjFqQ60W6az-Drj2%2BmOdQ%40mail.gmail.com
Thanks and Regards,
Shlok Kyal
