Re: new heapcheck contrib module

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>, Amul Sul <sulamul@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-08-28T13:58:43Z
Lists: pgsql-hackers
On Fri, Aug 28, 2020 at 1:07 AM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
> I was experimenting a bit with our internal heapcheck and found out that it's not helping with truncated CLOG anyhow.
> Will your module be able to gather tid's of similar corruptions?
>
> server/db M # select * from heap_check('pg_toast.pg_toast_4848601');
> ERROR:  58P01: could not access status of transaction 636558742
> DETAIL:  Could not open file "pg_xact/025F": No such file or directory.
> LOCATION:  SlruReportIOError, slru.c:913
> Time: 3439.915 ms (00:03.440)

This kind of thing gets really tricky. PostgreSQL uses errors in tons
of places to report problems, and if you want to accumulate a list of
errors and report them all rather than just letting the first one
cancel the operation, you need special handling for each individual
error you want to bypass. A tool like this naturally wants to use as
much PostgreSQL infrastructure as possible, to avoid duplicating a ton
of code and creating a bloated monstrosity, but all that code can
throw errors. I think the code in its current form is trying to be
resilient against problems on the table pages that it is actually
checking, but it can't necessarily handle gracefully corruption in
other parts of the system. For instance:

- CLOG could be truncated, as in your example
- the disk files could have had their permissions changed so that they
can't be accessed
- the PageIsVerified() check might fail when pages are read
- the TOAST table's metadata in pg_class/pg_attribute/etc. could be corrupted
- ...or the files for those system catalogs could've had their
permissions changed
- ....or they could contain invalid pages
- ...or their indexes could be messed up

I think there are probably a bunch more, and I don't think it's
practical to allow this tool to continue after arbitrary stuff goes
wrong. It'll be too much code and impossible to maintain. In the case
you mention, I think we should view that as a problem with clog rather
than a problem with the table, and thus out of scope.

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