Re: Introduce XID age based replication slot invalidation

Hi Bharath,

Do you think we need different GUCs for catalog_xmin and xmin? If table
bloat is a concern (not catalog bloat), then logical slots are not required
to invalidate unless the cluster is close to wraparound.

> I made the following design choice: try invalidating only once per
> vacuum cycle, not per table. While this keeps the cost of checking
> (incl. the XidGenLock contention) for invalidation to a minimum when
> there are a large number of tables and replication slots, it can be
> less effective when individual tables/indexes are large. Invalidating
> during checkpoints can help to some extent with the large table/index
> cases. But I'm open to thoughts on this.
>

It may not solve the intent when the vacuum cycle is longer, which one can
expect on a large database particularly when there is heavy bloat.

> Please find the attached patch for further review. I fixed the XID age
> calculation in ReplicationSlotIsXIDAged and adjusted the code
> comments.
>

I applied the patch and all the tests passed. A few comments:

@@ -495,7 +525,7 @@ vacuum(List *relations, const VacuumParams params,
BufferAccessStrategy bstrateg
MemoryContext vac_context, bool isTopLevel)
{
static bool in_vacuum = false;
-
+ static bool first_time = true;

first_time variable is not self explanatory, maybe something like
try_replication_slot_invalidation and add comments that it will be set to
false after the first check?

+ if (TransactionIdIsValid(xmin))
+ appendStringInfo(&err_detail, _("The slot's xmin %u exceeds the maximum
xid age %d specified by \"max_slot_xid_age\"."),
+ xmin,
+ max_slot_xid_age);

Slot invalidates even when the age is max_slot_xid_age, isn't it?

Thanks,
Satya