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

On Fri, Sep 25, 2026 at 2:28 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Fri, Sep 25, 2026 at 3:39 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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.

On the other hand, if the WAL has not even been flushed on the standby
yet, we skip the slot with SS_SKIP_WAL_NOT_FLUSHED before reaching
that point. The temporary slot is kept and we retry in the next cycle,
so we don't get stuck there.

>
> --I found that comments 1 and 2 in my previous email about set/reset
> of 'last_replayed_enable_lsn' are missed to be addressed in v2.
>
> --Also v2 does not apply through 'git am'.
>
> --I have a suggestion about comment improvement in
> synchronize_one_slot(), attached the patch. Please incorporate these
> changes if you agree.

Sorry I forgot to mention about comment 1 and 2; since the updated
patch renamed the field name to last_replayed_enable_lsn I think we
don't necessarily need to reset it at
UpdateLogicalDecodingStatusEndOfRecovery(). Also it renamed the
function name to StandbyLogicalDecodingEnabledSince() so it makes
sense to me to leave the field. As for comment 2, I think it's better
to have an shmem-init function for LogicalDecodingCtl rather than
initializing the new field in StartupLogicalDecodingStatus(). So I
prepared a patch for that (0001 patch).

I've incorporated your comment suggestions, and updated cosmetic
things.Please review them.

Regards,

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