On Tue, Oct 28, 2025 at 8:59 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Mon, Oct 27, 2025 at 11:56 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Oct 27, 2025 at 8:08 AM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > On Mon, Oct 27, 2025 at 6:12 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > >
> > > > Thanks for looking into it. I didn't get a chance to review the entire
> > > > 0002 but I looked at InvalidateObsoleteReplicationSlots() and have a
> > > > few questions related to that.
> > > >
> > > > In InvalidateObsoleteReplicationSlots(), the patch increments
> > > > n_valid_logicalslots before trying to invalidate the slot. Say, if
> > > > there is just one logical slot which got invalidated, then because we
> > > > have first incremented n_valid_logicalslots, how will it request to
> > > > disable logical_decoding after invalidating the last logical slot?
> > > >
> > >
> > > My initial understand on this:
> > >
> > > Whenever it invalidates a slot, the code jumps to the restart label,
> > > which in turn sets n_valid_logicalslots to 0. If it does not
> > > invalidate the slot but a logical slot exists, then
> > > n_valid_logicalslots remains greater than 0. Therefore, by the end of
> > > the function:
> > >
> > > --If valid logical slots were found and all were invalidated,
> > > n_valid_logicalslots must be 0.
> > > --If a valid logical slot was found but was not invalidated,
> > > n_valid_logicalslots must be greater than 0.
> > >
> > > But on looking again, I found that the code jumps to restart-label if
> > > lock-was released in interim. So can it happen that 'invalidated' is
> > > true but lock was not released by InvalidatePossiblyObsoleteSlot()
> > > causing n_valid_logicalslots to be greater than 0 even when the slot
> > > was actually invalidated?
> >
> > IIUC it cannot happen. When we invalidate one logical slot we release
> > the lwlock to avoid holding a lwlock while ReplicationSlotSave(). It's
> > not necessarily true that if we release the lwlock we invalidate the
> > slot but the opposite is always true.
> >
>
> Okay. Thanks for confirming. After looking more into the flow, I agree on this.
>
But, is it a good idea to rely on such a flag? I feel there should be
some other bullet-proof way to detect whether slot is invalidated so
that even if this internal API changes in future, we won't go wrong.
In any case, if we still want to go the same way at least a comment
explaining this rationale would be required.
--
With Regards,
Amit Kapila.
