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