public inbox for [email protected]help / color / mirror / Atom feed
test: avoid redundant standby catchup in 049_wait_for_lsn 7+ messages / 3 participants [nested] [flat]
* test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-17 12:25 Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Xuneng Zhou @ 2026-04-17 12:25 UTC (permalink / raw) To: pgsql-hackers <[email protected]>; Alexander Korotkov <[email protected]> Hi Alexander, Hackers, While working on adding more edge-case tests and fixing the timeline handling for WAIT FOR LSN, I noticed that the overall runtime of the test had increased by about 7 seconds since a8b61c23c5ff. I looked into the slowdown and found a potential source. Currently, the test creates the function, waits for the standby to catch up, tests it, then creates the procedure and waits for the standby to catch up again. Since both objects are only used by the same block of top-level statement checks, we can create them together in a single primary-side transaction and perform just one wait_for_catchup() before running both standby-side calls. This small TAP cleanup merges the creation of the PL/pgSQL wrapper function and procedure used for the top-level WAIT FOR checks in 049_wait_for_lsn.pl. 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. Patch attached. Thanks. -- Best, Xuneng Attachments: [application/x-patch] v1-0001-test-merge-wrapper-DDL-and-catchup-in-wait_for_ls.patch (1.8K, 2-v1-0001-test-merge-wrapper-DDL-and-catchup-in-wait_for_ls.patch) download | inline diff: From 5d67a5b2f37423e69f3a71d351feae349aaf25a9 Mon Sep 17 00:00:00 2001 From: alterego655 <[email protected]> Date: Fri, 17 Apr 2026 20:08:59 +0800 Subject: [PATCH v1] test: merge wrapper DDL and catchup in wait_for_lsn TAP 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. This avoids an extra 'wait_for_catchup()' on the delayed standby without changing the test coverage. --- src/test/recovery/t/049_wait_for_lsn.pl | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 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..11adcda1e9a 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -208,18 +208,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 +218,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.51.0 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-17 21:47 Michael Paquier <[email protected]> parent: Xuneng Zhou <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Michael Paquier @ 2026-04-17 21:47 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Alexander Korotkov <[email protected]> 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. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-18 04:19 Xuneng Zhou <[email protected]> parent: Michael Paquier <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Xuneng Zhou @ 2026-04-18 04:19 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers <[email protected]>; Alexander Korotkov <[email protected]> Hi Michael, 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. Best, Xuneng > ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-18 07:58 Alexander Korotkov <[email protected]> parent: Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Alexander Korotkov @ 2026-04-18 07:58 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: Michael Paquier <[email protected]>; pgsql-hackers <[email protected]> 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) ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-18 08:02 Alexander Korotkov <[email protected]> parent: Michael Paquier <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Alexander Korotkov @ 2026-04-18 08:02 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; pgsql-hackers <[email protected]> Hi, Michael! On Sat, Apr 18, 2026 at 12:47 AM Michael Paquier <[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 your observation. The intention of this test is to check explicit calls to WAIT FOR LSN. Yes, wait_for_catchup() now also internally calls WAIT FOR LSN. But checking wait_for_catchup() is not intention of this test, it's used in awfully a lot of other places. ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-20 10:21 Alexander Korotkov <[email protected]> parent: Alexander Korotkov <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Alexander Korotkov @ 2026-04-20 10:21 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: Michael Paquier <[email protected]>; pgsql-hackers <[email protected]> On Sat, Apr 18, 2026 at 10:58 AM Alexander Korotkov <[email protected]> wrote: > 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. Pushed. ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: test: avoid redundant standby catchup in 049_wait_for_lsn @ 2026-04-20 13:54 Xuneng Zhou <[email protected]> parent: Alexander Korotkov <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Xuneng Zhou @ 2026-04-20 13:54 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Michael Paquier <[email protected]>; pgsql-hackers <[email protected]> On Mon, Apr 20, 2026 at 6:21 PM Alexander Korotkov <[email protected]> wrote: > > On Sat, Apr 18, 2026 at 10:58 AM Alexander Korotkov > <[email protected]> wrote: > > 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. > > Pushed. > Thanks for pushing it. I haven't had time to investigate the latency yet, but will do it later. Best, Xuneng ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-04-20 13:54 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-17 12:25 test: avoid redundant standby catchup in 049_wait_for_lsn Xuneng Zhou <[email protected]> 2026-04-17 21:47 ` Michael Paquier <[email protected]> 2026-04-18 04:19 ` Xuneng Zhou <[email protected]> 2026-04-18 07:58 ` Alexander Korotkov <[email protected]> 2026-04-20 10:21 ` Alexander Korotkov <[email protected]> 2026-04-20 13:54 ` Xuneng Zhou <[email protected]> 2026-04-18 08:02 ` Alexander Korotkov <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox