Re: Restrict copying of invalidated replication slots

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Restrict copying of invalidated replication slots
Date: 2025-03-18 09:26:22
Message-ID: CANhcyEV+wwhquX3J9J5gKVdhirGFhNxDAXJArz3udp94vNsvPA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 17 Mar 2025 at 12:18, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Mar 10, 2025 at 11:46 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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

Attachment Content-Type Size
master_v8-0001-Restrict-copying-of-invalidated-replication-slots.patch application/octet-stream 5.5 KB
REL_17_v8-0001-Restrict-copying-of-invalidated-replication-slots.patch application/octet-stream 5.6 KB
REL_16_v8-0001-Restrict-copying-of-invalidated-replication-slots.patch application/octet-stream 5.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ilia Evdokimov 2025-03-18 09:26:48 Re: [PERF] Improve Cardinality Estimation for Joins with GROUP BY Having Single Clause
Previous Message Hayato Kuroda (Fujitsu) 2025-03-18 09:17:21 RE: pg_recvlogical requires -d but not described on the documentation