Horton

Re: new heapcheck contrib module

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>, Peter Geoghegan <pg@bowt.ie>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Stephen Frost <sfrost@snowman.net>, Michael Paquier <michael@paquier.xyz>, Amul Sul <sulamul@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-10-26T13:37:34Z
Lists: pgsql-hackers 
On Fri, Oct 23, 2020 at 2:04 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Seems to work, so I pushed it (after some compulsive fooling
> about with whitespace and perltidy-ing).  It appears to me that
> the code coverage for verify_heapam.c is not very good though,
> only circa 50%.  Do we care to expend more effort on that?

There are two competing goods here. On the one hand, more test
coverage is better than less. On the other hand, finicky tests that
have platform-dependent results or fail for strange reasons not
indicative of actual problems with the code are often judged not to be
worth the trouble. An early version of this patch set had a very
extensive chunk of Perl code in it that actually understood the page
layout and, if we adopt something like that, it would probably be
easier to test a whole bunch of scenarios. The downside is that it was
a lot of code that basically duplicated a lot of backend logic in
Perl, and I was (and am) afraid that people will complain about the
amount of code and/or the difficulty of maintaining it. On the other
hand, having all that code might allow better testing not only of this
particular patch but also other scenarios involving corrupted pages,
so maybe it's wrong to view all that code as a burden that we have to
carry specifically to test this; or, alternatively, maybe it's worth
carrying even if we only use it for this. On the third hand, as Mark
points out, if we get 0002 committed, that will help somewhat with
test coverage even if we do nothing else.

Thanks for committing (and adjusting) the patches for the existing
buildfarm failures. If I understand the buildfarm results correctly,
hornet is still unhappy even after
321633e17b07968e68ca5341429e2c8bbf15c331?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Add pg_amcheck, a CLI for contrib/amcheck.

  2. Refactor and generalize the ParallelSlot machinery.

  3. Generalize parallel slot result handling.

  4. Move some code from src/bin/scripts to src/fe_utils to permit reuse.

  5. Factor pattern-construction logic out of processSQLNamePattern.

  6. Doc: clean up verify_heapam() documentation.

  7. Fix more portability issues in new amcheck code.

  8. Fix portability issues in new amcheck test.

  9. Try to avoid a compiler warning about using fxid uninitialized.

  10. Extend amcheck to check heap pages.

  11. Adjust walsender usage of xlogreader, simplify APIs

  12. Improve checking of child pages in contrib/amcheck.

  13. Sanitize line pointers within contrib/amcheck.

  14. Fix possible sorting error when aborting use of abbreviated keys.