public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Xuneng Zhou <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: test: avoid redundant standby catchup in 049_wait_for_lsn
Date: Sat, 18 Apr 2026 10:58:42 +0300
Message-ID: <CAPpHfdvSgasQ3XeTAbA=cqH_BVY9pD6BVuXj+xEgyYGx2_nfEw@mail.gmail.com> (raw)
In-Reply-To: <CABPTF7VouG5DSvdgPSg1ko6ybHhNPQk5sxJQMiy8r_gr_bEtug@mail.gmail.com>
References: <CABPTF7WZ1yuYz8V=xsbghg8e7qaAm5MpyNw6BthWcbN7+P6biw@mail.gmail.com>
<[email protected]>
<CABPTF7VouG5DSvdgPSg1ko6ybHhNPQk5sxJQMiy8r_gr_bEtug@mail.gmail.com>
Hi, Xuneng.
On Sat, Apr 18, 2026 at 7:20 AM Xuneng Zhou <[email protected]> wrote:
>> On Fri, Apr 17, 2026 at 08:25:35PM +0800, Xuneng Zhou wrote:
>> > The change preserves the same coverage while removing one redundant
>> > replay catch-up on the delayed standby. It appears to reduce the test
>> > runtime by about 7 seconds, though I have looked into why much of the
>> > improvement comes from this change alone.
>>
>> Alexander may think differently and remove that, but I disagree. The
>> test is clearly written so as we want two wait checks to happen, for
>> for CREATE FUNCTION, and one for CREATE PROCEDURE. Removing the first
>> check to keep only the second one removes its meaning. In short, I
>> see nothing wrong to deal with here.
>
>
> Thank you for the review. I agree that the two wait checks serve distinct purposes and are not redundant. The main motivation for this patch was efficiency. In my testing, the new test added approximately 7 seconds to the runtime, while the creation of the procedure and function completed quickly. I suspect the latency stems from the wait-for-catch-up step. When I removed it, the test runtime dropped by about 7 seconds.I haven't yet investigated why the wait is so costly in this case. I should probably look into that before proposing this change.
On my laptop the time needed to run t/049_wait_for_lsn.pl also drops
from 20 secs to 12 secs. The influence to the runtime of the whole
test suite in parallel would be not that big as CPU time only drops
from 2.16 sec to 2.07 sec. But anyway that's pretty significant.
I've revised comment message a bit and surrounding comments. I'm
going to push this if no objections.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v2-0001-049_wait_for_lsn.pl-create-function-and-procedure.patch (3.0K, 2-v2-0001-049_wait_for_lsn.pl-create-function-and-procedure.patch)
download | inline diff:
From b85c48eb9edaca42065b9596d87ad4c6ac0e9eb1 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 18 Apr 2026 10:52:19 +0300
Subject: [PATCH v2] 049_wait_for_lsn.pl: create function and procedure at once
Create the PL/pgSQL function and procedure for the top-level WAIT FOR
checks in a single transaction, then wait once for standby replay before
running both tests. Also revise some surrounding comments.
This avoids an extra 'wait_for_catchup()' on the delayed standby without
changing the test coverage.
Discussion: https://postgr.es/m/CABPTF7WZ1yuYz8V%3Dxsbghg8e7qaAm5MpyNw6BthWcbN7%2BP6biw%40mail.gmail.com
Author: Xuneng Zhou <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
---
src/test/recovery/t/049_wait_for_lsn.pl | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 8358c57f7b7..0e74175f9eb 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -172,8 +172,8 @@ ok($output eq "timeout", "WAIT FOR returns correct status after timeout");
# 5. Check mode validation: standby modes error on primary, primary mode errors
# on standby, and primary_flush works on primary. Also check that WAIT FOR
-# triggers an error if called within another function or inside a transaction
-# with an isolation level higher than READ COMMITTED.
+# triggers an error if called within a function, procedure, anonymous DO block,
+# or inside a transaction with an isolation level higher than READ COMMITTED.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -200,6 +200,8 @@ ok( $stderr =~
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
+# should error
$node_primary->safe_psql(
'postgres', qq[
CREATE FUNCTION pg_wal_replay_wait_wrap(target_lsn pg_lsn) RETURNS void AS \$\$
@@ -208,18 +210,7 @@ CREATE FUNCTION pg_wal_replay_wait_wrap(target_lsn pg_lsn) RETURNS void AS \$\$
END
\$\$
LANGUAGE plpgsql;
-]);
-
-$node_primary->wait_for_catchup($node_standby);
-$node_standby->psql(
- 'postgres',
- "SELECT pg_wal_replay_wait_wrap('${lsn3}');",
- stderr => \$stderr);
-ok($stderr =~ /WAIT FOR can only be executed as a top-level statement/,
- "get an error when running within a function");
-$node_primary->safe_psql(
- 'postgres', qq[
CREATE PROCEDURE pg_wal_replay_wait_proc(target_lsn pg_lsn) AS \$\$
BEGIN
EXECUTE format('WAIT FOR LSN %L;', target_lsn);
@@ -229,6 +220,13 @@ LANGUAGE plpgsql;
]);
$node_primary->wait_for_catchup($node_standby);
+$node_standby->psql(
+ 'postgres',
+ "SELECT pg_wal_replay_wait_wrap('${lsn3}');",
+ stderr => \$stderr);
+ok($stderr =~ /WAIT FOR can only be executed as a top-level statement/,
+ "get an error when running within a function");
+
$node_standby->psql(
'postgres',
"CALL pg_wal_replay_wait_proc('${lsn3}');",
--
2.39.5 (Apple Git-154)
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: test: avoid redundant standby catchup in 049_wait_for_lsn
In-Reply-To: <CAPpHfdvSgasQ3XeTAbA=cqH_BVY9pD6BVuXj+xEgyYGx2_nfEw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox