Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

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:
> >
> > On Sat, Oct 25, 2025 at 4:06 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > On Fri, Oct 24, 2025 at 4:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > 5.
> > > > +bool
> > > > +CheckLogicalSlotExists(void)
> > > > {
> > > > …
> > > > + /* NB: counting invalidated slots */
> > > > +
> > > > + if (SlotIsLogical(s))
> > > >
> > > > Why can't we avoid counting invalid slots? I think this needs more
> > > > comments. BTW, shouldn't this patch consider changing
> > > > effective_wal_level when the last logical slot is invalidated?
> > > > Ideally, when logical decoding is not possible in the system, then we
> > > > can reduce the wal_level back to replica, no?
> > >
> > > Hmm, good point. I've considered this idea before but I didn't
> > > implement it probably because it makes the code more complex. But
> > > thinking of this idare carefully, it doesn't seem too complex. I've
> > > implemented this part as a separate patch to make reviews easy. I'll
> > > merge them if it looks good.
> > >
> >
> > 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.

>
> > Another related point is that, say we decide to disable decoding
> > because the last logical slot got invalidated and
> > RequestDisableLogicalDecoding()->LogicalDecodingStatusChangeAllowed()
> > returns false, then how the disabling will happen?
> >
>
> If LogicalDecodingStatusChangeAllowed() returns false, we do not
> disable logical decoding because that essentially means
> recovery-in-progress and if that is the case, we do not allow
> status-change. LogicalDecodingStatusChangeAllowed() returns false
> always on standby until it is promoted and recovery ends.

Exactly.

Regards,

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