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

On Sat, Sep 26, 2026 at 3:54 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Sep 25, 2026 at 2:28 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Fri, Sep 25, 2026 at 3:39 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > > 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.
> >
> > I agree with the problem and solution, but I don't think ths slot will
> > later be skipped with 'SS_SKIP_WAL_NOT_FLUSHED' as the WALs are
> > already flushed; it is the replay which is slow and that check
> > compares against GetStandbyFlushRecPtr(), not replay position I think
> > it will wait somewhere in
> > LogicalSlotAdvanceAndCheckSnapState()-->read_local_xlog_page_guts as
> > 'wait_for_wal' is true and standy then waits for replay to happen. If
> > my understanding is correct, slotsync will be stuck on that one slot
> > untli replays happen, but let's see what Nisha has found in her tests.
> > I might be wrong too.
>
> You're right, if only the replay is delayed, it can end up waiting in
> read_local_xlog_page_guts(). But I think this is pre-existing behavior
> for a remote slot whose confirmed_lsn is ahead of the standby's replay
> position.
>

I confirmed the wait with a test. With replay paused on the standby, a
new failover slot 's' whose position is ahead of replay is created on
the standby as temporary. The slotsync worker then waits in
read_local_xlog_page_guts() for replay to catch up. Meanwhile, a
second slot 't', advanced on the primary only up to an LSN the standby
had already replayed, is not synced until replay is resumed.

Call stack of the slotsync worker:

pg_usleep
read_local_xlog_page_guts
read_local_xlog_page
XLogReadRecord
LogicalSlotAdvanceAndCheckSnapState
update_local_synced_slot
update_and_persist_local_synced_slot
synchronize_one_slot
synchronize_slots
ReplSlotSyncWorkerMain
...

I see the same wait on HEAD without the patch too, so I agree that
this is pre-existing.

--
Thanks,
Nisha