Re: Fix "unexpected logical decoding status change" error; from concurrent logical decoding activation

On Mon, Jul 27, 2026 at 9:36 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Fri, Jul 24, 2026 at 12:20 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > The other observation I had while looking at this patch was: On a
> > standby, InvalidateObsoleteReplicationSlots() can disable logical
> > decoding whenever it invalidates the last valid logical slot via
> > following check:
> >
> > if (invalidated_logical && !found_valid_logicalslot)
> > RequestDisableLogicalDecoding();
> > RequestDisableLogicalDecoding() isn't guarded against recovery, so a
> > purely local invalidation (RS_INVAL_HORIZON/WAL_REMOVED/IDLE_TIMEOUT)
> > makes the standby disable decoding even though the primary still has
> > it enabled. The standby's status is supposed to follow the primary via
> > XLOG_LOGICAL_DECODING_STATUS_CHANGE replay, and there's no
> > self-healing since the primary never sends an "enable". So, won't that
> > be a problem because after that no new slots will be allowed to be
> > created on standby and slotsync also won't be able perform sync. Am, I
> > missing something? If not then probable the above check needs
> > additional check: "!RecoveryInProgress()".
>
> In this case, the checkpointer will try to disable logical decoding
> but does nothing as it's still in recovery. See the
> RecoveryInProgress() check in DisableLogicalDecodingIfNecessary().
>
> If we skip calling RequestDisableLogicalDecoding() in this case, we
> would end up missing the disable request as it can be interleaved with
> the promotion process. Imagine:
>
> 1. the startup process begins the promotion, decides the
> new_status=true as there is a logical slot (in
> UpdateLogicalDecodingStatusEndOfRecovery()).
> 2. a backend process drops the last logical slot and skips calling
> RequestDisableLogicalDecoding() as the recovery status is still
> in-progress.
> 3. the startup completes the
> UpdateLogicalDecodingStatusEndOfRecovery(). The logical decoding is
> enabled even though there is no logical slot.
>

Thanks, I missed the check in DisableLogicalDecodingIfNecessary(). But
will it be better to add a comment atop that check and also atop
RequestDisableLogicalDecoding()? If so, how about something like
attached?

--
With Regards,
Amit Kapila.