Horton

Re: pg_amcheck contrib application

Mark Dilger <mark.dilger@enterprisedb.com>

View thread View on postgr.es
From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>, Thomas Munro <thomas.munro@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, "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: 2021-03-12T22:24:18Z
Lists: pgsql-hackers

Attachments


> On Mar 12, 2021, at 1:43 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> On Fri, Mar 12, 2021 at 2:31 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> I'll commit something shortly to address these.
> 
> There are some interesting failures in the test cases on the
> buildfarm. One of the tests ($offnum == 13) corrupts the TOAST pointer
> with a garbage value, expecting to get the message "final toast chunk
> number 0 differs from expected value 6". But on florican and maybe
> other systems we instead get "final toast chunk number 0 differs from
> expected value 5". That's because the value of TOAST_MAX_CHUNK_SIZE
> depends on MAXIMUM_ALIGNOF. I think that on 4-byte alignment systems
> it works out to 2000 and on 8-byte alignment systems it works out to
> 1996, and the value being stored is 10000 bytes, hence the problem.
> The place where the calculation goes different seems to be in
> MaximumBytesPerTuple(), where it uses MAXALIGN_DOWN() on a value that,
> according to my calculations, will be 2038 on all platforms, but the
> output of MAXALIGN_DOWN() will be 2032 or 2036 depending on the
> platform. I think the solution to this is just to change the message
> to match \d+ chunks instead of exactly 6. We should do that right away
> to avoid having the buildfarm barf.
> 
> But, I also notice a couple of other things I think could be improved here:
> 
> 1. amcheck is really reporting the complete absence of any TOAST rows
> here due to a corrupted va_valueid. It could pick a better phrasing of
> that message than "final toast chunk number 0 differs from expected
> value XXX". I mean, there is no chunk 0. There are no chunks at all.
> 
> 2. Using SSSSSSSSS as the perl unpack code for the varlena header is
> not ideal, because it's really 2 1-byte fields followed by 4 4-byte
> fields. So I think you should be using CCllLL, for two unsigned bytes
> and then two signed 4-byte quantities and then two unsigned 4-byte
> quantities. I think if you did that you'd be overwriting the
> va_valueid with the *same* garbage value on every platform, which
> would be better than different ones. Perhaps when we improve the
> message as suggested in (1) this will become a live issue, since we
> might choose to say something like "no TOAST entries for value %u".
> 
> -- 
> Robert Haas
> EDB: http://www.enterprisedb.com


This does nothing to change the verbiage from contrib/amcheck, but it should address the problems discussed here in pg_amcheck's regression tests.

Commits

  1. amcheck: Improve some confusing reports about TOAST problems.

  2. amcheck: Reword some messages and fix an alignment problem.

  3. amcheck: fix multiple problems with TOAST pointer validation

  4. amcheck: Remove duplicate XID/MXID bounds checks.

  5. amcheck: Fix verify_heapam's tuple visibility checking rules.

  6. nbtree VACUUM: Cope with buggy opclasses.

  7. Improve pg_amcheck's TAP test 003_check.pl.

  8. Fix a confusing amcheck corruption message.

  9. Doc: add note about how to run the pg_amcheck regression tests.

  10. In pg_amcheck tests, don't depend on perl's Q/q pack code.

  11. pg_amcheck: Keep trying to fix the tests.

  12. pg_amcheck: Try to fix still more test failures.

  13. Try to avoid apparent platform-dependency in IPC::Run

  14. Fix portability issues in pg_amcheck's 004_verify_heapam.pl.

  15. Try to fix compiler warnings.

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

  17. Refactor and generalize the ParallelSlot machinery.

  18. Remove old-style VACUUM FULL (which was known for a little while as