From d8b4985f88f579942cb1c13e67f146fb32d76ba9 Mon Sep 17 00:00:00 2001 From: Shveta Malik Date: Fri, 25 Sep 2026 14:48:15 +0530 Subject: [PATCH] comment change --- src/backend/replication/logical/slotsync.c | 51 ++++++++++++---------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index a07118c30a7..715ef6439bc 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -868,32 +868,39 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid remote_dbid, true); /* - * 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 - * deactivation could not invalidate our slot because it did not exist - * yet, and WAL following the 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 remote slot's restart_lsn can predate a status change record + * this standby has already replayed: the primary drops the last + * logical slot, possibly recreating it with the same name, after + * fetch_remote_slots() ran. The resulting deactivation record could + * not invalidate our slot, since it didn't exist yet, so WAL + * following the (stale) remote restart_lsn may lack the information + * logical decoding needs. Checking only whether logical decoding is + * currently enabled is not enough, since it may have been disabled + * and re-enabled since the remote slot information was fetched. * - * 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, so replay cannot get past that record behind our - * back. That is also why the status needs no recheck before the slot - * is persisted. (The invalidation is performed only in hot standby, - * which slot synchronization requires anyway.) + * The check must run after ReplicationSlotCreate(), which makes the + * slot visible and acquired. From this point on, a deactivation + * record 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. * - * WAL beyond the replay position tells us nothing, so a remote - * restart_lsn past it is accepted and left to the interlock above. + * 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. * * 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. + * one, so a slot that would have been usable may still be dropped; + * the next cycle fetches fresh remote information. The slot can't be + * kept in that case, since it would go on using the stale + * restart_lsn. */ replay_lsn = GetXLogReplayRecPtr(NULL); if (remote_slot->restart_lsn <= replay_lsn && -- 2.34.1