Re: Restrict copying of invalidated replication slots

On Tue, Feb 25, 2025 at 7:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> 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?

Good point. I think it's possible.

> 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.

The wal_status of the newly created slot would immediately become
'lost' and the next checkpoint will invalidate it. Do we need to do
something to deal with this case?

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com