pgsql: Teach verify_heapam() to validate update chains within a page.

Started by Robert Haasover 3 years ago4 messagescomitters

robertmhaas@gmail.com

over 3 years ago

Teach verify_heapam() to validate update chains within a page.

Prior to this commit, we only consider each tuple or line pointer
on the page in isolation, but now we can do some validation of a line
pointer against its successor. For example, a redirect line pointer
shouldn't point to another redirect line pointer, and if a tuple
is HOT-updated, the result should be a heap-only tuple.

Himanshu Upadhyaya and Robert Haas, reviewed by Aleksander Alekseev,
Andres Freund, and Peter Geoghegan.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/bbc1376b39627c6bddd8a0dc0a7dda24c91a97a0

Modified Files
--------------
contrib/amcheck/verify_heapam.c | 291 +++++++++++++++++++++++++++++-
src/bin/pg_amcheck/t/004_verify_heapam.pl | 250 +++++++++++++++++++++++--
2 files changed, 524 insertions(+), 17 deletions(-)

Andrew Dunstan

andrew@dunslane.net

over 3 years ago

In reply to: Robert Haas (#1)

Re: pgsql: Teach verify_heapam() to validate update chains within a page.

On 2023-03-22 We 09:17, Robert Haas wrote:

Teach verify_heapam() to validate update chains within a page.

Prior to this commit, we only consider each tuple or line pointer
on the page in isolation, but now we can do some validation of a line
pointer against its successor. For example, a redirect line pointer
shouldn't point to another redirect line pointer, and if a tuple
is HOT-updated, the result should be a heap-only tuple.

Himanshu Upadhyaya and Robert Haas, reviewed by Aleksander Alekseev,
Andres Freund, and Peter Geoghegan.

This has apparently broken one case in the buildfarm, which now fails an
upgrade from REL9_2_STABLE because we run amcheck after the upgrade:

heap table "regression.public.xacttest", block 0, offset 1:
    non-heap-only update produced a heap-only tuple at offset 18
heap table "regression.public.xacttest", block 0, offset 2:
    non-heap-only update produced a heap-only tuple at offset 19
heap table "regression.public.xacttest", block 0, offset 4:
    non-heap-only update produced a heap-only tuple at offset 20
heap table "regression.public.xacttest", block 0, offset 5:
    non-heap-only update produced a heap-only tuple at offset 21
heap table "regression.public.wslot", block 0, offset 89:
    non-heap-only update produced a heap-only tuple at offset 95
heap table "regression.public.pslot", block 0, offset 95:
    non-heap-only update produced a heap-only tuple at offset 100
heap table "regression.public.pslot", block 0, offset 96:
    non-heap-only update produced a heap-only tuple at offset 101

I don't know if this is a bug in this commit or if the REL9_2_STABLE
repo is really broken.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

Andres Freund

andres@anarazel.de

over 3 years ago

In reply to: Andrew Dunstan (#2)

Re: pgsql: Teach verify_heapam() to validate update chains within a page.

Hi,

On 2023-03-22 15:33:15 -0400, Andrew Dunstan wrote:

On 2023-03-22 We 09:17, Robert Haas wrote:

Teach verify_heapam() to validate update chains within a page.

Prior to this commit, we only consider each tuple or line pointer
on the page in isolation, but now we can do some validation of a line
pointer against its successor. For example, a redirect line pointer
shouldn't point to another redirect line pointer, and if a tuple
is HOT-updated, the result should be a heap-only tuple.

Himanshu Upadhyaya and Robert Haas, reviewed by Aleksander Alekseev,
Andres Freund, and Peter Geoghegan.

This has apparently broken one case in the buildfarm, which now fails an
upgrade from REL9_2_STABLE because we run amcheck after the upgrade:

heap table "regression.public.xacttest", block 0, offset 1:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 18
heap table "regression.public.xacttest", block 0, offset 2:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 19
heap table "regression.public.xacttest", block 0, offset 4:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 20
heap table "regression.public.xacttest", block 0, offset 5:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 21
heap table "regression.public.wslot", block 0, offset 89:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 95
heap table "regression.public.pslot", block 0, offset 95:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 100
heap table "regression.public.pslot", block 0, offset 96:
ï¿½ï¿½ï¿½ non-heap-only update produced a heap-only tuple at offset 101

I don't know if this is a bug in this commit or if the REL9_2_STABLE repo is
really broken.

FWIW, I get errors on a bunch of catalog tables, on HEAD, if I run the
regression tests and then run amcheck.

heap table "ecpg1_regression.pg_catalog.pg_class", block 7, offset 16:
non-heap-only update produced a heap-only tuple at offset 19
heap table "ecpg1_regression.pg_catalog.pg_class", block 13, offset 49:
non-heap-only update produced a heap-only tuple at offset 52
heap table "ecpg1_regression.pg_catalog.pg_class", block 13, offset 53:
...
heap table "regression_pg_surgery.pg_catalog.pg_class", block 0, offset 4:
non-heap-only update produced a heap-only tuple at offset 5
heap table "regression_pg_surgery.pg_catalog.pg_class", block 0, offset 5:
non-heap-only update produced a heap-only tuple at offset 6

So either this very quickly bagged the first bugs, or there's something wrong
with the checks.

Greetings,

Andres Freund

andres@anarazel.de

over 3 years ago

In reply to: Andres Freund (#3)

Re: pgsql: Teach verify_heapam() to validate update chains within a page.

Hi,

On 2023-03-22 13:05:49 -0700, Andres Freund wrote:

So either this very quickly bagged the first bugs, or there's something wrong
with the checks.

I'll reply on the thread with a few more details / issues.

/messages/by-id/CA+TgmoZub-bCg=ORmbcCCbteQ2UQUc14T2GK+v+6-VLd-8yK4g@mail.gmail.com

Greetings,

Andres Freund