Re: Fix "unexpected logical decoding status change" error; from concurrent logical decoding activation - Mailing list pgsql-hackers

On Thu, Sep 24, 2026 at 4:41 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Wed, Sep 23, 2026 at 1:45 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Sep 21, 2026 at 11:32 PM Nikolay Samokhvalov <nik@postgres.ai> wrote:
> > >
> > > On Thu, Jul 16, 2026 at 6:52 AM Masahiko Sawada
> > > <sawada.mshk@gmail.com> wrote:
> > > > For slot synchronization, the local slot could be created and
> > > > persisted based on the remote slot information fetched before the
> > > > deactivation was replayed, leaving a valid slot whose restart_lsn
> > > > precedes the deactivation. Decoding such a slot after a failover fails
> > > > with:
> > > >
> > > > ERROR: unexpected logical decoding status change 0
> > > >
> > > > These races are confined to the narrow window between checking the
> > > > logical decoding status and the new slot becoming visible; once the
> > > > slot is visible, the invalidation performed by the deactivation
> > > > already covers it. So the fix is simple: re-check the logical decoding
> > > > status after the new slot becomes visible. Regular slot creation
> > > > raises an error and slot synchronization skips persisting the slot. If
> > > > the deactivation happens after the recheck instead, it is guaranteed
> > > > to invalidate the now-visible slot as usual. The attached 0002
> > > > implements this.
> > >
> > > The disable/re-enable case described in the comment above the final
> > > IsLogicalDecodingEnabled() check in update_and_persist_local_synced_slot() is
> > > reachable.
> > >
> > > On b73d13c3, the reproducer uses this sequence:
> > >
> > > 1. Slot sync fetches failover slot S and pauses at
> > > replication-slot-create-begin, before creating the local slot.
> > > 2. The primary drops S. The standby replays the logical-decoding
> > > deactivation while no local S exists to invalidate.
> > > 3. The primary recreates S. The standby replays the reactivation.
> > > 4. The old slot sync resumes with the first incarnation's restart_lsn.
> > >
> > > The final IsLogicalDecodingEnabled() check now returns true, so the old slot
> > > information is persisted. After promoting the standby, decoding that slot
> > > fails with:
> > >
> > > ERROR: unexpected logical decoding status change 0
> >
> > Thank you for the report. Yes, while the window is very short in
> > practice, it indeed happens if the logical decoding is disabled and
> > re-enabled (by dropping and creating the same name failover slot)
> > between the slotsync worker fetches the slot information and creates
> > it.
> >
> > It actually hits my concern mentioned in the comment in
> > update_and_persist_local_synced_slot():
> >
> >      * XXX: this check cannot detect the case where logical decoding is
> >      * already re-enabled by a slot creation on the primary at this point.
> >      * Detecting that would require comparing the slot's restart_lsn with the
> >      * LSN at which logical decoding was last enabled.
> >
> > > The attached patch adds a logical-decoding status generation. Slot sync
> > > records it before fetching remote slot information and refuses to persist a
> > > new slot if the generation changed in the meantime. It drops the temporary
> > > slot so that the next attempt fetches the current incarnation.
> >
> >  Thank you for the patch.
> >
> > An alternative approach that I think is better is to have the LSN of
> > the last replayed status change record in LogicalDecodingCtlData, and
> > check if logical decoding has been enabled since the remote slot's
> > restart_lsn. That's simpler than the proposed approach as we don't
> > need to increment the generation counter at both activation and
> > deactivation (which is not necessary outside recovery), nor to add
> > logical_decoding_generation to RemoteSlot. It also checks what we
> > actually need, that is, whether the WAL from the restart_lsn can be
> > decoded, rather than whether the status changed while synchronizing
> > slots.
> >
> > Also, I think it's better to move the check to right after
> > ReplicationSlotCreate() in synchronize_one_slot() because (1) it can
> > simplify the code flow as we don't need to care about the slot dropped
> > in update_and_persist_local_synced_slot(), (2) it can save the WAL
> > reservation and the xmin_horizon computation, and (3) IIUC with the
> > proposed patch, the check can be bypassed when
> > update_and_persist_local_synced_slot() returns early due to
> > slotsync_skip_reason, leaving a temporary slot with the stale
> > restart_lsn. Once the slot passes the check right after its creation,
> > a later deactivation invalidates the slot, so we don't need to check
> > it again before persisting the slot.
> >
> > I've attached the patch.
> >
>
> I tested the patch and it fixes the problem. I found no critical
> issues. A couple of comments:
> 1) Now that a newly created synced slot is dropped on a failed new
> check rather than kept as RS_TEMPORARY, a standby that is lagging in
> replay can end up creating and dropping the slot on every sync cycle.
> For example, replay is paused with pg_wal_replay_pause() or
> recovery_min_apply_delay is large. After the primary turns logical
> decoding off and then on again, the standby receives the activation
> record but doesn't replay it. Meanwhile the slotsync worker keeps
> fetching the failover slot, creates it, fails the new
> IsLogicalDecodingEnabledSince() check, and drops it. This repeats
> every cycle until the record is replayed.
>
> Each cycle creates the slot on disk and a pgstat entry, then removes
> both again. I think this can be avoided with a cheaper pre-check,
> IsLogicalDecodingEnabledSince(remote_slot->restart_lsn), before
> ReplicationSlotCreate().
>
> Thoughts?

I agree with your analysis. I think that in this case, the logical
slot doesn't need to be dropped because WAL records after its
restart_lsn are written with logical decoding information. Thinking on
IsLogicalDecodingEnabledSince() further, I think it can work fine for
the slot only when the replay LSN >= slot's restart_lsn. If the slot's
restart_lsn > replay_lsn, we can leave the slot. Such a slot will be
skipped for SS_SKIP_WAL_NOT_FLUSHED anyway. That way, the slot would
have to be recreated only in the disable/re-enable case.

>
> 2) The overview comment at the top of slotsync.c explains each reason
> a slot isn't synced yet, but it doesn't mention this new
> drop-and-retry case. Should we add it there too?

The patch describes the details of the disabled/re-enabled case in
synchronize_one_slot() and it looks sufficient to me.

I've updated the patch for the above idea. In this version, the patch
tracks only the last STATUS_CHANGE record that enables logical
decoding, which makes it easy to check if logical decoding has
continuously been enabled since the remote slot's restart_lsn.

Regards,

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