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

On Sat, Sep 26, 2026 at 2:44 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Sep 25, 2026 at 6:25 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I've incorporated your comment suggestions, and updated cosmetic
> > things.Please review them.
> >
>
> + /*
> + * The remote slot information can predate a status change record that
> + * this standby has already replayed. That happens when the last
> + * logical slot on the primary is dropped, and possibly re-created
> + * with the same name, after fetch_remote_slots() ran. The resulting
> + * deactivation could not invalidate our slot because it did not exist
> + * yet, and WAL following the (stale) remote restart_lsn may lack the
> + * information logical decoding needs. Checking only whether logical
> + * decoding is enabled is not enough, as it can have been disabled and
> + * enabled again in the meantime.
> + *
> + * The check has to come after ReplicationSlotCreate(), which makes
> + * the slot both visible and acquired. A deactivation replayed from
> + * here on finds the slot in InvalidatePossiblyObsoleteSlot(), signals
> + * a recovery conflict and waits for the slot to be released before
> + * invalidating it (only in hot standby, which slot synchronization
> + * requires anyway). Replay therefore cannot get past that record
> + * behind our back, so the slot never needs to be rechecked before
> + * being persisted.
> + *
> + * The check only runs once replay has reached the remote restart_lsn;
> + * otherwise it is skipped and the slot is kept as-is. Without this, a
> + * standby lagging behind the primary (replay paused, or a large
> + * recovery_min_apply_delay) could fetch a live, valid restart_lsn
> + * from the primary and have it rejected by
> + * StandbyLogicalDecodingEnabledSince(), whose answer reflects only
> + * WAL replayed so far and says nothing about an LSN replay hasn't
> + * reached yet. That would drop a perfectly good slot every cycle.
> + *
> + * Even so, the comparison uses the remote restart_lsn rather than the
> + * local one, so a slot that would have been usable may be dropped;
> + * the next cycle fetches fresh information. The slot cannot be kept,
> + * as it would go on using the stale restart_lsn.
> + */
> + replay_lsn = GetXLogReplayRecPtr(NULL);
> + if (remote_slot->restart_lsn <= replay_lsn &&
> + !StandbyLogicalDecodingEnabledSince(remote_slot->restart_lsn))
> + {
> + ereport(LOG,
> + errmsg("could not synchronize replication slot \"%s\"",
> + remote_slot->name),
> + errdetail("Logical decoding was disabled after the remote slot's
> restart LSN %X/%08X.",
> + LSN_FORMAT_ARGS(remote_slot->restart_lsn)));
> +
> + ReplicationSlotDropAcquired(false);
> +
> + if (slot_persistence_pending)
> + *slot_persistence_pending = true;
> +
> + return false;
> + }
> +
> /* For shorter lines. */
> slot = MyReplicationSlot;
>
> It is not clear from comments why it is okay to proceed when
> remote_slot->restart_lsn > replay_lsn? Because if it is possible to
> persist the slot in that case then the above issue can hit later say
> if the promotion happens. I think it is not possible to persist the
> slot and if that is the case, then we can capture it in comments on
> the lines: (The check is skipped until replay reaches the remote
> restart_lsn, as StandbyLogicalDecodingEnabledSince() describes
> replayed WAL only and would otherwise reject a valid restart_lsn from
> a lagging standby. Skipping it lets no bad slot through, as the slot
> is not persisted in this cycle anyway. It starts out with the remote
> restart_lsn, see reserve_wal_for_local_slot(), and
> update_local_synced_slot() can neither find a serialized snapshot at
> an LSN this standby has not replayed nor build one by decoding WAL it
> has not replayed. It therefore reports no consistent snapshot, the
> slot stays temporary, and a later cycle retries it once replay has
> advanced.).

Such a slot won't be persisted before replay catches up, but I think
the reason is slightly different from what you described, and it
doesn't wait for a later cycle. When remote_slot->restart_lsn >
replay_lsn, the check is skipped and we reach
read_local_xlog_page_guts() via LogicalSlotAdvanceAndCheckSnapState(),
where we wait for the replay LSN to catch up to the slot's
confirmed_lsn. It doesn't report "no consistent snapshot". We wait
there, and the slot can then reach a consistent snapshot and be
persisted in the same cycle. The patch doesn't change any of this. The
check on replay_lsn is there so that the new check stays a no-op when
it
has nothing to say about the given LSN.

The reason it's okay to proceed is that we created and acquired the
slot before the wait. If a STATUS_CHANGE record that disables logical
decoding is replayed while we are waiting, the slot invalidation finds
our slot, signals a recovery conflict and waits for us to release it
before invalidating it, and slotsync worker is terminated. So no bad
slot is left behind.

> If the above reasoning is correct it doesn't seem like a good idea to
> split the safety of the above mechanism in different functions.
> Instead, we can move the new check just before
> update_and_persist_local_synced_slot() and then avoid relying on the
> code in update_and_persist_local_synced_slot() that can persist the
> slot.

Does it mean that we return early before calling
update_and_persist_local_synced_slot() if replay_lsn < restart_lsn? If
so, I think it would change the existing behavior rather than fix this
issue.

Regards,

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