Horton

Re: new heapcheck contrib module

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>
Date: 2020-04-29T18:41:04Z
Lists: pgsql-hackers 
On Wed, Apr 22, 2020 at 10:43 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> It's simple enough to extend the tap test a little to check for those things.  In v3, the tap test skips tests if the page size is not 8k, and also if the tuples do not fall on the page where expected (which would happen due to alignment issues, gremlins, or whatever.).

Skipping the test if the tuple isn't in the expected location sounds
really bad. That will just lead to the tests passing without actually
doing anything. If the tuple isn't in the expected location, the tests
should fail.

> There are other approaches, though.  The HeapFile/HeapPage/HeapTuple perl modules recently submitted on another thread *could* be used here, but only if those modules are likely to be committed.

Yeah, I don't know if we want that stuff or not.

> This test *could* be extended to autodetect the page size and alignment issues and calculate at runtime where tuples will be on the page, but only if folks don't mind the test having that extra complexity in it.  (There is a school of thought that regression tests should avoid excess complexity.). Do you have a recommendation about which way to go with this?

How much extra complexity are we talking about? It feels to me like
for a heap page, the only things that are going to affect the position
of the tuples on the page -- supposing we know the tuple size -- are
the page size and, I think, MAXALIGN, and that doesn't sound too bad.
Another possibility is to use pageinspect's heap_page_items() to
determine the position within the page (lp_off), which seems like it
might simplify things considerably. Then, we're entirely relying on
the backend to tell us where the tuples are, and we only need to worry
about the offsets relative to the start of the tuple.

I kind of like that approach, because it doesn't involve having Perl
code that knows how heap pages are laid out; we rely entirely on the C
code for that. I'm not sure if it'd be a problem to have a TAP test
for one contrib module that uses another contrib module, but maybe
there's some way to figure that problem out.

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