Horton

Re: new heapcheck contrib module

Mark Dilger <mark.dilger@enterprisedb.com>

View thread View on postgr.es
From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.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-23T04:21:11Z
Lists: pgsql-hackers

Attachments


> On Oct 22, 2020, at 7:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Mark Dilger <mark.dilger@enterprisedb.com> writes:
>> Ahh, crud.  It's because
>> 	syswrite($fh, '\x77\x77\x77\x77', 500)
>> is wrong twice.  The 500 was wrong, but the string there isn't the bit pattern we want -- it's just a string literal with backslashes and such.  It should have been double-quoted.
> 
> Argh.  So we really have, using same test except
> 
> 	memcpy(&lp, "\\x77", sizeof(lp));
> 
> little endian:	off = 785c, flags = 2, len = 1b9b
> big endian:	off = 2e3c, flags = 0, len = 3737
> 
> which explains the apparent LP_DEAD result.
> 
> I'm not particularly on board with your suggestion of "well, if it works
> sometimes then it's okay".  Then we have no idea of what we really tested.
> 
> 			regards, tom lane

Ok, I've pruned it down to something you may like better.  Instead of just checking that *some* corruption occurs, it checks the returned corruption against an expected regex, and if it fails to match, you should see in the logs what you got vs. what you expected.

It only corrupts the first two line pointers, the first one with 0x77777777 and the second one with 0xAAAAAAAA, which are consciously chosen to be bitwise reverses of each other and just strings of alternating bits rather than anything that could have a more complicated interpretation.

On my little-endian mac, the 0x77777777 value creates a line pointer which redirects to an invalid offset 0x7777, which gets reported as decimal 30583 in the corruption report, "line pointer redirection to item at offset 30583 exceeds maximum offset 38".  The test is indifferent to whether the corruption it is looking for is reported relative to the first line pointer or the second one, so if endian-ness matters, it may be the 0xAAAAAAAA that results in that corruption message.  I don't have a machine handy to test that.  It would be nice to determine the minimum amount of paranoia necessary to make this portable and not commit the rest.

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.