Hi,
On Fri, Sep 25, 2026 at 6:09 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Sep 24, 2026 at 4:41 AM Nisha Moond <nisha.moond412@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.
The patch looks good to me overall, but I have a question for the check of
replay position:
+ replay_lsn = GetXLogReplayRecPtr(NULL);
+ if (remote_slot->restart_lsn <= replay_lsn &&
If the goal is to avoid retrying when the standby is lagging, couldn't we
simply avoid setting slot_persistence_pending = true in this case? The original
slot is dropped anyway, and any slot re-created later is a different one. This
seems more similar to the SS_SKIP_INVALID case, where we don't retry even if a
re-created slot exists. I think that would also make the logic simpler.
Best Regards,
Zhijie Hou
