Re: new heapcheck contrib module
Mark Dilger <mark.dilger@enterprisedb.com>
> On Apr 29, 2020, at 11:41 AM, Robert Haas <robertmhaas@gmail.com> wrote: > > 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? The page size is easy to query, and the test already does so, skipping if the answer isn't 8k. The test could recalculate offsets based on the pagesize rather than skipping the test easily enough, but the MAXALIGN stuff is a little harder. I don't know (perhaps someone would share?) how to easily query that from within a perl test. So the test could guess all possible alignments that occur in the real world, read from the page at the offset that alignment would create, and check if the expected datum is there. The test would have to be careful to avoid false positives, by placing data before and after the datum being checked with bit patterns that cannot be misinterpreted as a match. That level of complexity seems unappealing, at least to me. It's not hard to write, but maintaining stuff like that is an unwelcome burden. > 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. Yeah, I'll give this a try. — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Commits
-
Add pg_amcheck, a CLI for contrib/amcheck.
- 9706092839db 14.0 landed
-
Refactor and generalize the ParallelSlot machinery.
- f71519e545a3 14.0 landed
-
Generalize parallel slot result handling.
- 418611c84d00 14.0 landed
-
Move some code from src/bin/scripts to src/fe_utils to permit reuse.
- e955bd4b6c2b 14.0 landed
-
Factor pattern-construction logic out of processSQLNamePattern.
- 2c8726c4b0a4 14.0 landed
-
Doc: clean up verify_heapam() documentation.
- 4c49d8fc15ee 14.0 landed
-
Fix more portability issues in new amcheck code.
- 321633e17b07 14.0 landed
-
Fix portability issues in new amcheck test.
- 860593ec3bd1 14.0 landed
-
Try to avoid a compiler warning about using fxid uninitialized.
- 8bb0c9770e80 14.0 landed
-
Extend amcheck to check heap pages.
- 866e24d47db1 14.0 landed
-
Adjust walsender usage of xlogreader, simplify APIs
- 850196b610d2 13.0 cited
-
Improve checking of child pages in contrib/amcheck.
- d114cc538715 13.0 cited
-
Sanitize line pointers within contrib/amcheck.
- a9ce839a3137 12.0 cited
-
Fix possible sorting error when aborting use of abbreviated keys.
- 008c4135ccf6 10.0 cited