On Wed, Sep 23, 2026 at 1:45 AM Masahiko Sawada <[email protected]> wrote: > > On Mon, Sep 21, 2026 at 11:32 PM Nikolay Samokhvalov <[email protected]> wrote: > > > > On Thu, Jul 16, 2026 at 6:52 AM Masahiko Sawada > > <[email protected]> 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? 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? -- Thanks, Nisha
