Re: Restrict copying of invalidated replication slots

On Tue, Feb 25, 2025 at 11:21 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Feb 25, 2025 at 2:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > >
> > > Scenario-3: the source gets invalidated after creating the copied slot
> > > (i.e. after create_logical/physical_replication_slot()). In this case,
> > > since the newly copied slot have the same restart_lsn as the source
> > > slot, both slots are invalidated.
> > >
> >
> > Which part of the code will cover Scenario-3? Shouldn't we give ERROR
> > for Scenario-3 as well?
>
> In scenario-3, the backend process executing
> pg_copy_logical/physical_replication_slot() already holds the new
> copied slot and its restart_lsn is the same or older than the source
> slot's restart_lsn. Therefore, if the source slot is invalidated at
> that timing, the copied slot is invalidated too, resulting in an error
> by the backend.
>

AFAICU, InvalidateObsoleteReplicationSlots() is not serialized with
this operation. So, isn't it possible that the source slot exists at
the later position in ReplicationSlotCtl->replication_slots and the
loop traversing slots is already ahead from the point where the newly
copied slot is created? If so, the newly created slot won't be marked
as invalid whereas the source slot will be marked as invalid. I agree
that even in such a case, at a later point, the newly created slot
will also be marked as invalid.

--
With Regards,
Amit Kapila.