Re: Introduce XID age based replication slot invalidation

Hi Nisha,

Thanks for the detailed response.

> If the invalidation is sticky, the recovery path for these slots is
> also blocked. Once the worker recovers or is rerun,
> synchronize_one_slot() will just keep skipping the slot at the "Skip
> the sync of an invalidated slot" check (SS_SKIP_INVALID), since
> nothing else clears a locally invalidated synced slot, so it can never
> be recreated with fresh state.
>

Agreed, and I'd missed that the drop in drop_local_obsolete_slots() is the
only thing that ever clears a locally invalidated synced slot.
Making it sticky would leave the slot skipped at SS_SKIP_INVALID
indefinitely, and since ReplicationSlotDrop() refuses synced slots while in
recovery there's nothing on the standby either. So please disregard that
suggestion.

> Also, I think the same stuck state can already be reached when
> "max_slot_wal_keep_size" is set only on the standby and the remote
> slot's restart_lsn is not advancing. (I haven't reproduced this
> though.)

I think you're right, and the code path lines up,
reserve_wal_for_local_slot() sets the recreated slot's restart_lsn to
max(remote restart_lsn, min_safe_lsn), where min_safe_lsn is the lesser of
the redo pointer and the minimum slot LSN. After a WAL-based invalidation
the recreated slot therefore lands ahead of the remote, and
update_local_synced_slot() skips it on the restart_lsn half of the same
condition the catalog_xmin case trips.

> The comments above drop_local_obsolete_slots() already
> anticipate standby-side invalidation from max_slot_wal_keep_size, but
> assume the slot gets recreated successfully in the next cycle, which
> doesn't hold when the remote isn't advancing.
>

If the two are equivalent, then that comment looks like the concrete thing
worth fixing, since it's the assumption that misleads. A note in the docs
that a synced slot can be invalidated on the standby, and that recovering
it needs action on the primary, might help too, given users can't drop it
themselves.

Also, regarding both GUCs, Once the recreated slot is stuck, its frozen
restart_lsn or catalog_xmin eventually trips the limit again, and because
ReplicationSlotRelease()
leaves active_proc set on temporary slots, each subsequent invalidation
terminates the slot sync worker rather than just marking the slot. If
that's considered acceptable, fine by me. I mainly wanted to make sure
it was a known consequence rather than a surprise.

Regards,
Surya Poondla