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
