Re: Introduce XID age based replication slot invalidation

Hi,

On Thu, Mar 26, 2026 at 2:50 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Thank you for updating the patch. I've reviewed the patch and have
> some review comments:

Thank you for reviewing the patch.

> + /* translator: %s is a GUC variable name */
> + appendStringInfo(&err_detail, _("The slot's xmin
> %u at next transaction ID %u exceeds the age %d specified by
> \"%s\"."),
> + xmin,
> + nextXID,
> + max_slot_xid_age,
> + "max_slot_xid_age");
>
> I think it's better to show the age of the slot's xmin instead of the
> recent XID.

Agreed.

> ---
> +
> + if (!TransactionIdIsNormal(oldestXmin) || !TransactionIdIsNormal(nextXID))
> + return false;
> +
>
> Do we expect that the passed oldestXmin or nextXID could be non-normal
> XIDs? I think the function assumes these are valid XIDs.

The oldestXmin is now removed. Please see the responses at the end.

> Also, since this function is called only by heap_vacuum_rel(), we can
> call ReadNextTransactionId() within this function.

Agreed.

> ---
> + if (IsReplicationSlotXIDAged(slot_xmin, slot_catalog_xmin, nextXID))
>
> We compute the cutoff XID in IsReplicationSlotXIDAged() again, which
> seems redundant.
>
> I've attached the fixup patch addressing these comments and having
> some code cleanups. Please review it.

The fixup patch looked good to me, I had that merged in the attached v5 patch.

> I'm reviewing the regression test part, and will share review comments soon.
>
> > I've also attached the 0002 patch that adds a test case to demo a
> > production-like scenario by pushing the database to XID wraparound
> > limits and checking if the XID-age based invalidation with the GUC
> > setting at the default vacuum_failsafe_age of 1.6B works correctly,
> > and whether autovacuum can successfully remove this replication slot
> > blocker to proceed with freezing and bring the database back to
> > normal. I don't intend to get this committed unless others think
> > otherwise, but I wanted to have this as a reference.
>
> Thank you for sharing the test script! I'll check it as well.

Thank you.

On Thu, Mar 26, 2026 at 3:42 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram(at)gmail(dot)com> wrote:
>
> Hi,
>
> + if (InvalidateXIDAgedReplicationSlots(vacrel->cutoffs.OldestXmin,
> + ReadNextTransactionId()))
>
> Does this account catalog xmin for data tables?

Nice catch! When vacuum runs on regular tables, it doesn't cover
catalog_xmin in the OldestXmin. So if catalog_xmin is blocking
relfrozenxid advancement, slot invalidation doesn't happen. I updated
vacuum_get_cutoffs to return slot_catalog_xmin and slot_xmin. These
values are already available in ComputeXidHorizons, so this doesn't
require an additional proc-array lock.

I also added support for XID age based slot invalidation during
checkpoints. This helps standbys that can have replication slots but
where vacuum doesn't run. (It skips synced slots, just like
idle_replication_slot_timeout does.)

Please find the attached v5 patches for further review. Thank you!

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com