Horton

Re: XLogReadRecord() error in XlogReadTwoPhaseData()

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Thomas Munro <thomas.munro@gmail.com>, pgbf@twiska.com, Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-01-24T08:02:43Z
Lists: pgsql-hackers 
On Sun, Jan 23, 2022 at 06:34:32PM -0800, Andres Freund wrote:
> On 2022-01-23 18:10:07 -0800, Noah Misch wrote:
> > On Sun, Jan 23, 2022 at 05:40:54PM -0800, Andres Freund wrote:
> > > Test::more's description: "If it's something the programmer hasn't done yet,
> > > use TODO. This is for any code you haven't written yet, or bugs you have yet
> > > to fix, but want to put tests in your testing script (always a good idea)."
> >
> > Could do that.  Every run that doesn't get the flaky failure will print a
> > message like "TODO passed:  3-5", though the test file could mitigate that by
> > declaring the TODO only on configurations where we expect a failure.
> 
> Yea, that's what I was thinking we'd do.
> 
> > The
> > 027_stream_regress.pl trouble involves reaching a die(), not failing a test in
> > this sense, so that one would take more work.
> 
> I'm really not a perl person... But my understanding is that todo_skip() would
> address this? I.e. something like
> 
> TODO:
> {
>    $todo_skip "linux/sparc has unaddressed problems with partial page  overwrites"
>      if ($^O eq 'linux' and $Config{archname'} ~= 'sparc');
> 
>    ok(whatever is broken);
> }

Yes.  todo_skip() behaves much like regular skip().  The enclosed tests don't
run.  Hence, it prevents die() and BAIL_OUT() failures.  $TODO is a different
beast; tests still run, and it changes the reporting.  For 003_cic_2pc.pl, I'm
fine using $TODO so we continue to run all test commands and quietly log their
results.  For 027_stream_regress.pl, which would need deep changes to use
$TODO, it works to use any of todo_skip, skip, or skip_all.  I prefer
skip_all, because it prints the skip reason to gmake's stdout.  (If the number
of affected users is zero as theorized, the choice doesn't matter.)  Any
objections?  Here's the appearance of each strategy on gmake's stdout:

=== todo_skip
[23:56:45] t/003_cic_2pc.pl .. ok       60 ms ( 0.00 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.05 CPU)
[23:56:45]
All tests successful.
Files=1, Tests=5,  1 wallclock secs ( 0.01 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.06 CPU)
Result: PASS

=== skip
[23:55:47] t/003_cic_2pc.pl .. ok       59 ms ( 0.00 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.05 CPU)
[23:55:48]
All tests successful.
Files=1, Tests=5,  1 wallclock secs ( 0.01 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.06 CPU)
Result: PASS

=== skip_all
[23:31:04] t/003_cic_2pc.pl .. skipped: filesystem bug
[23:31:04]
Files=1, Tests=0,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.07 CPU)
Result: NOTESTS

=== $TODO, test 1 is expected fail, tests 2-5 are unexpected pass
[23:32:32] t/003_cic_2pc.pl .. ok     1371 ms ( 0.01 usr  0.00 sys +  0.51 cusr  0.24 csys =  0.76 CPU)
[23:32:33]
All tests successful.

Test Summary Report
-------------------
t/003_cic_2pc.pl (Wstat: 0 Tests: 5 Failed: 0)
  TODO passed:   2-5
Files=1, Tests=5,  1 wallclock secs ( 0.02 usr  0.00 sys +  0.51 cusr  0.24 csys =  0.77 CPU)
Result: PASS

Commits

  1. Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.

  2. Under has_wal_read_bug, skip contrib/bloom/t/001_wal.pl.

  3. Use Test::Builder::todo_start(), replacing $::TODO.

  4. On sparc64+ext4, suppress test failures from known WAL read failure.

  5. Report any XLogReadRecord() error in XlogReadTwoPhaseData().