Re: POC: enable logical decoding when wal_level = 'replica' without a server restart - Mailing list pgsql-hackers

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?

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

thanks
Shveta