Re: Introduce XID age based replication slot invalidation

Hi Bharath,

Please find the attached v14 patches. 0003 now adds support for

invalidating XID-aged synced replication slots on standbys.

v14 applies cleanly and builds clean on the master. I think 0003 has a
problem when sync_replication_slots is on: the invalidation doesn't stick,
and the slot ends up worse off than before.

With max_slot_xid_age = 100 on the standby only, and a failover logical
slot on the primary with no consumer so its catalog_xmin stays at 695,
consuming 200 XIDs and checkpointing both nodes gives:

LOG: invalidating obsolete replication slot "failover_slot"
DETAIL: The slot's catalog xmin age of 200 exceeds the configured
"max_slot_xid_age" of 100 by 100 transactions.
LOG: dropped replication slot "failover_slot" of database with OID 5
LOG: could not synchronize replication slot "failover_slot" [repeats]

The slot was temporary=false synced=true catalog_xmin=695 beforehand, and
ends up temporary=true synced=true invalidation_reason=NULL
catalog_xmin=895.

The restartpoint invalidates it, then drop_local_obsolete_slots() drops it,
since local_sync_slot_required() returns false for a slot "invalidated
while the corresponding remote slot is still valid". synchronize_one_slot()
re-creates it as RS_TEMPORARY with a fresh catalog_xmin from
GetOldestSafeDecodingTransactionId() (895), and
update_local_synced_slot() then won't sync it, because the remote's
catalog_xmin, 695, precedes it. That recovery path works for the existing
invalidation causes because the remote is normally being consumed and soon
advances past the standby, but max_slot_xid_age targets slots whose horizon
is not advancing.

The primary's horizon does get released, the catalog_xmin on the physical
slot the standby streams from advanced from 695 to 895. The
cost is the synced slot: it doesn't become sync-ready again, and
ReplicationSlotCleanup() drops the worker's temporary slots when it exits,
so it is gone at promotion. invalidation_reason = 'xid_aged' doesn't
persist either, so there's nothing left to alert on.

ReplicationSlotRelease() clears active_proc only for RS_PERSISTENT slots,
so the recreated slot stays active (active_pid was
the slotsync worker itself), its frozen catalog_xmin ages out again, and
the next restartpoint takes the active branch:

LOG: terminating process 45192 to release replication slot
"failover_slot"
LOG: slot sync worker started

After which the slot is recreated at catalog_xmin=1095. So roughly every
max_slot_xid_age transactions, the checkpointer SIGTERMs the slot sync
worker. None of this shows up in Testcase 6, which sets
sync_replication_slots = off and drives the sync by hand, the only
configuration in which nothing resurrects the slot.

Would it make sense to keep the invalidation sticky for this cause, i.e.
have local_sync_slot_required() not drop a synced slot invalidated with
RS_INVAL_XID_AGE? The operator would see it, the horizon stays released,
and neither the slot nor the worker gets churned. The alternative is Amit's
earlier suggestion of relying on the primary invalidating its own slot, but
that doesn't cover your cases 1 and 2.

Regards,
Surya Poondla