Re: Fix src/test/subscription/t/029_on_error.pl test when wal_debug is enabled

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Ilyasov Ian <ianilyasov(at)outlook(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix src/test/subscription/t/029_on_error.pl test when wal_debug is enabled
Date: 2024-05-15 22:13:20
Message-ID: ZkUzgHfiWx9Eh3YA@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, May 15, 2024 at 05:58:18PM +0530, Amit Kapila wrote:
> I guess it could be more work if we want to enhance the test for
> ERRORs other than the primary key violation.

And? You could pass the ERROR string expected as argument of the
function if more flexibility is wanted at some point, no? It happens
that you don't require that now, which is fine for the three scenarios
calling test_skip_lsn.

> One simple fix is to
> update the log_offset to the location of the LOG after successful
> replication of un-conflicted data. For example, the Log location after
> we execute the below line in the test:
>
> # Check replicated data
> my $res =
> $node_subscriber->safe_psql('postgres', "SELECT
> count(*) FROM tbl");
> is($res, $expected, $msg);

That still looks like a shortcut to me, weak to race conditions on
slow machines where more log entries could be generated in-between.
So it seems to me that you'd still want to make sure that the CONTEXT
from which the LSN is retrieved maps to the sole expected error. This
is not going to be stable unless there are stronger checks to avoid
log entries that can parasite the output, and a stronger matching
ensures that.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2024-05-15 22:43:06 Re: Why is citext/regress failing on hamerkop?
Previous Message Andres Freund 2024-05-15 22:03:19 Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM