[PATCH] Speed up pg_waldump TAP test and fix some GitHub CI Windows flakiness

On Fri, Aug 28, 2026 at 11:43:54PM -0400, Sehrope Sarkuni wrote:

Was testing a rebased Windows patch using GitHub CI and had some flaky slow
test timeout errors for an unrelated pg_waldump test. On Windows (and only
Windows) it was running for 500+ seconds and timing out. After a bit of
research, I figured it's because of the notoriously slow spoofed IPC that
perl does for the forked pg_waldump invocations (that then gets grepped by
perl).

Your simple patch is also proving to make my runs of this test sort of
40% faster on Linux, at quick glance.

The --limit=1 additions are no-brainers in the commands where we don't
expect specific outputs and/or records. Same thing can be said for
the few --limit=5 cases, where I guess you've just chosen a number to
be representative enough for the output. This number looks fine here.

-               '--start' => $start_lsn
+               '--start' => $contrecord_lsn

Hmm. Ok here. That's a nice trick. Perhaps this should have a
comment explaining why not the start_lsn but the contrecord LSN
because it's cheaper with less records?

-       my @lines = test_pg_waldump($path, $start_lsn, $end_lsn);
-       is(grep(!/^rmgr: \w/, @lines), 0, 'all output lines are rmgr lines');
-
-       @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn);
+       my @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn); 

Does this removal imply a loss of coverage?

+ @lines = test_pg_waldump($path, $start_lsn, $end_lsn,
'--fullpage', '--limit' => 5);

A limit of 5 for the fullpage case should be OK. Why not.

+       # --limit must come before a bare --stats: src/port/getopt_long.c,
+       # used on Windows, skips the argument after an optional_argument
+       # option given without "=".  Drop the ordering once that is fixed.

Or fix the order here, without the comment. You have created already
a thread for the other issue with the options, let's just make this
code clean from the start so as we don't come back to it.

For the last case with "only lines for selected block", perhaps raise
a bit the limit for more output?
--
Michael

Hi,

On Mon, 31 Aug 2026 at 10:53, Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Aug 28, 2026 at 11:43:54PM -0400, Sehrope Sarkuni wrote:

Was testing a rebased Windows patch using GitHub CI and had some flaky slow
test timeout errors for an unrelated pg_waldump test. On Windows (and only
Windows) it was running for 500+ seconds and timing out. After a bit of
research, I figured it's because of the notoriously slow spoofed IPC that
perl does for the forked pg_waldump invocations (that then gets grepped by
perl).

Your simple patch is also proving to make my runs of this test sort of
40% faster on Linux, at quick glance.

Yes, thank you for the patch! I confirm that runtime dropped from ~400
seconds to ~100 seconds on Windows CI.

The --limit=1 additions are no-brainers in the commands where we don't
expect specific outputs and/or records. Same thing can be said for
the few --limit=5 cases, where I guess you've just chosen a number to
be representative enough for the output. This number looks fine here.

Doesn't '--limit N' decrease coverage? For example,

`
@lines =
test_pg_waldump($path, $start_lsn, $end_lsn, '--rmgr' => 'Btree');
is(grep(!/^rmgr: Btree/, @lines), 0, 'only Btree lines');
`

was checking all Btree records from $start to $end. However,

`
@lines = test_pg_waldump(
$path, $start_lsn, $end_lsn,
'--rmgr' => 'Btree',
'--limit' => 5);
is(grep(!/^rmgr: Btree/, @lines), 0, 'only Btree lines');
`

checks the first 5 Btree records. Is that enough?

Also, if I am not mistaken, after this patch there is no test that
confirms pg_waldump runs correctly from start to end.

-       my @lines = test_pg_waldump($path, $start_lsn, $end_lsn);
-       is(grep(!/^rmgr: \w/, @lines), 0, 'all output lines are rmgr lines');
-
-       @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn);
+       my @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn);

Does this removal imply a loss of coverage?

I think you are right. At least, we lose coverage of tar and gzip inputs.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Mon, Aug 31, 2026 at 01:43:22PM +0300, Nazir Bilal Yavuz wrote:

Doesn't '--limit N' decrease coverage? For example,

Yeah, I am worried about that for tests with dedicated outputs
checked.

@lines =
test_pg_waldump($path, $start_lsn, $end_lsn, '--rmgr' => 'Btree');
is(grep(!/^rmgr: Btree/, @lines), 0, 'only Btree lines');
`
was checking all Btree records from $start to $end. However,

Also, if I am not mistaken, after this patch there is no test that
confirms pg_waldump runs correctly from start to end.

I'd still want that once, yes.

-       my @lines = test_pg_waldump($path, $start_lsn, $end_lsn);
-       is(grep(!/^rmgr: \w/, @lines), 0, 'all output lines are rmgr lines');
-
-       @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn);
+       my @lines = test_pg_waldump($path, $contrecord_lsn, $end_lsn);

Does this removal imply a loss of coverage?

I think you are right. At least, we lose coverage of tar and gzip inputs.

As far as I remember my read of the patch yesterday, various --limit=1
additions don't really matter because we do not care about checking
dedicated output patterns: we just want to make a quick check that some
command pattern works. If we only include these, how much time are we
able to cut? I'd guess that it would still be a nice chunk, but I did
not test.. And we could backpatch that as well.
--
Michael

Hi,

On Tue, 1 Sept 2026 at 11:23, Michael Paquier <michael@paquier.xyz> wrote:

As far as I remember my read of the patch yesterday, various --limit=1
additions don't really matter because we do not care about checking
dedicated output patterns: we just want to make a quick check that some
command pattern works. If we only include these, how much time are we
able to cut? I'd guess that it would still be a nice chunk, but I did
not test.. And we could backpatch that as well.

Here is v2, which adds '--limit=1' to tests that check the command
pattern works. Commit message is updated accordingly.

- On my Linux computer, total time of 10 of 'pg_waldump/001_basic'
tests is reduced from 33s to 28s.

- On Windows CI it is reduced from 430s to 200s.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Thanks for the reviews!

I had put together a branch with some of feedback (comment clean up,
explicit full run through, etc). But seems like it'll be better to
split it up to allow for some backpatching so scrapped that and
went with the attached.

For the Windows testing I'm stuck with GitHub CI so it's kind of
a pain to cycle through it. Been using the Linux runtimes, total bytes,
and decode counts as a proxy to get some faster feedback.

Latest split out patches are attached. The first one does just the
`--limit=...` where we are only looking for one row. The second one
keeps track of the range of LSN to have smarter start / stop positions.
The third one goes a step further moves some of the bigger pieces out
of the loop so they are not repeated. The comments are cleaned up
as well.

Here's the timings / counts running on Linux:

time stdout decoded per decoded
(mean) via IPC archive scenario total
master 4.46s 139.1 MB 109.8 MB 393.5 MB
patch1 3.70s 57.7 MB 91.1 MB 273.4 MB
patch2 3.33s 37.1 MB 42.6 MB 127.8 MB
patch3 2.51s 19.4 MB 10.8 MB 64.2 MB

Each row includes the patches above it. Patch 1 caps the probe
runs via --limit, patch 2 bounds the scans with marker LSNs,
patch 3 decodes once per check instead of once per scenario.

Running in GitHub CI the Windows runtime twice for those are:

Part 1 - 769s, 247s
Part 2 - 128s, 95s
Part 3 - 121s, 178s

It's a _lot_ of noise in CI, but I think 1 + 2 is already the bulk of the gains.
Look like that's a bit further than Nazir's version as it picks a more
specific LSN for the ranges.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

Hi,

On Tue, 1 Sept 2026 at 17:04, Sehrope Sarkuni <sehrope@jackdb.com> wrote:

It's a _lot_ of noise in CI, but I think 1 + 2 is already the bulk of the gains.
Look like that's a bit further than Nazir's version as it picks a more
specific LSN for the ranges.

I haven't checked the patches yet but for this problem I generally
select any other long-running test and try to normalize test timings.
What I mean is:

With patch:

postgresql:pg_waldump/001_basic 200s
pg_rewind/002_databases 455s

Without the patch:

pg_waldump/001_basic 300s
pg_rewind/002_databases 315s

After normalizing the 'without the patch' timings:

pg_waldump/001_basic 430s
pg_rewind/002_databases 455s

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Tue, Sep 1, 2026 at 11:37 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

I haven't checked the patches yet but for this problem I generally
select any other long-running test and try to normalize test timings.

Oh that's a good idea. I'll try it like that next to get a better read.

In the meantime, I tried your v2 on the same linux box v.s. the rest:

time stdout decoded per decoded
(mean) via IPC archive scenario total
master 4.51s 139.1 MB 109.8 MB 393.5 MB
v2 3.78s 37.5 MB 74.2 MB 222.5 MB
part1 3.74s 57.7 MB 91.1 MB 273.4 MB
part2 3.19s 37.1 MB 42.6 MB 127.8 MB
part3 2.52s 19.4 MB 10.8 MB 64.2 MB

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

On Tue, Sep 01, 2026 at 12:32:50PM -0400, Sehrope Sarkuni wrote:

In the meantime, I tried your v2 on the same linux box v.s. the rest:

time stdout decoded per decoded
(mean) via IPC archive scenario total
master 4.51s 139.1 MB 109.8 MB 393.5 MB
v2 3.78s 37.5 MB 74.2 MB 222.5 MB
part1 3.74s 57.7 MB 91.1 MB 273.4 MB
part2 3.19s 37.1 MB 42.6 MB 127.8 MB
part3 2.52s 19.4 MB 10.8 MB 64.2 MB

The differences between v2 and part1 are the two switches of
$start_lsn to $contrecord_lsn, which don't matter as we want to check
the end of the records. These moves cut already a lot of activity
without touching coverage.

Let's just do Nazir's v2 as a first step with a backpatch to ease the
CI and buildfarm pain a bit. Will apply that portion in a bit, that's
worth its own weight in gold.
--
Michael

On Tue, Sep 1, 2026 at 6:38 PM Michael Paquier <michael@paquier.xyz> wrote:

The differences between v2 and part1 are the two switches of
$start_lsn to $contrecord_lsn, which don't matter as we want to check
the end of the records. These moves cut already a lot of activity
without touching coverage.

Let's just do Nazir's v2 as a first step with a backpatch to ease the
CI and buildfarm pain a bit.

Sounds good.

Will apply that portion in a bit, that's worth its own weight in gold.

Thanks again to you both for looking at this. Was an interesting rabbit
hole to learn about why it's so much slower for Windows.

Will take another look at this later to see if if there's more to be
gained or similar low hanging fruit elsewhere in the suite.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/