Re: Introduce XID age based replication slot invalidation

Hi,

On Wed, Apr 1, 2026 at 12:39 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I've reviewed the v7 patch and have some review comments:

Thank you for reviewing the patch.

> +# Advance the given number of XIDs
> +sub advance_xids
> +{
> +   my ($node, $nxids) = @_;
> +   my $sql = join(";\n", ("SELECT pg_current_xact_id()") x $nxids);
> +   $node->safe_psql('postgres', $sql);
> +}
>
> I think we can create a procedure on primary5 instance to consume XIDs
> as follow:
>
> $standby5->safe_psql(
>    'postgres',
>    qq{CREATE PROCEDURE consume_xid(cnt int)
> AS \$\$
> DECLARE
>     i int;
>     BEGIN
>         FOR i in 1..cnt LOOP
>             EXECUTE 'SELECT pg_current_xact_id()';
>             COMMIT;
>         END LOOP;
>     END;
> +\$\$
> LANGUAGE plpgsql;
> });

Agreed. Although the test timings don't improve (9 seconds on my dev
machine) after moving to the procedure vs. sending pg_current_xact_id
SQL statements, the procedure approach looks better and is more
consistent.

> ---
> +# Create a subscriber node
> +my $subscriber5 = PostgreSQL::Test::Cluster->new('subscriber5');
> +$subscriber5->init(allows_streaming => 'logical');
> +$subscriber5->start;
>
> Do we really need to create a subscriber for this test? I think we can
> simply create a logical slot on the primary5 and test the XID-age
> based slot invalidation.

Nice catch! Removed.

> ---
> I've attached a fixup patch to propose some cleanup and refactoring, including:
>
> - changes to invalidation errdetail message.
> - passing xidLimit instead of recentXid to simplify the invalidation logic.
> - documentation changes.
> - comment changes.

I took the above changes into v8 and fixed a typo in using xidLimit
instead of slotXidLimit.

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

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