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-24T02:10:07Z
Lists: pgsql-hackers

Attachments

On Sun, Jan 23, 2022 at 05:40:54PM -0800, Andres Freund wrote:
> On 2022-01-23 17:17:59 -0800, Noah Misch wrote:
> > On Sun, Jan 23, 2022 at 05:03:04PM -0800, Andres Freund wrote:
> > > On January 23, 2022 3:29:27 PM PST
> > > >(a) Modify the tests so the affected animals can skip affected tests by
> > > >setting an environment variable, named PG_TEST_HAS_WAL_READ_BUG or similar.
> > > 
> > > Why not just detect the problem in the tap test and skip, rather than requiring multiple buildfarm configs to be changed as well as the test itself? 
> > 
> > End users running PostgreSQL test suites to acceptance-test their stack should
> > consider the affected stack unusable for PostgreSQL.
> 
> I'd bet that that's zero users ;)

Wouldn't surprise me.  I'm attaching what I had written and discarded.  If
nobody else hates it, I can live with it.

> > Hence, I ruled out that
> > approach, despite having implemented it at one point.  Under some plausible
> > set of goals, it is optimal.
> 
> It's not perfect due to the way we run our tests (seeing output is hard, it's
> not aggregated), but marking the test as todo rather than SKIP seems like the
> most appropriate test status. It's known to be a problem, we've not fixed it,
> but we want to be able to run the tests.
> 
> 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.  The
027_stream_regress.pl trouble involves reaching a die(), not failing a test in
this sense, so that one would take more work.

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().