Re: amcheck: add index-all-keys-match verification for B-Tree - Mailing list pgsql-hackers

From Wenbo Lin
Subject Re: amcheck: add index-all-keys-match verification for B-Tree
Date
Msg-id ba92ac77-24f1-44ad-abf0-11e64e0a7831@gmail.com
Whole thread Raw
In response to Re: amcheck: add index-all-keys-match verification for B-Tree  (Andrey Borodin <x4mmm@yandex-team.ru>)
List pgsql-hackers
Hi Andrey
I've been reviewing the patch and ran some concurrent tests against it.
I found an issue with false positive corruption reports under concurrent
VACUUM.

+ slot = table_slot_create(state->heaprel, NULL);
+ found = table_tuple_fetch_row_version(state->heaprel, tid,
+                         SnapshotAny, slot);
+ if (!found)
+ {
+         ExecDropSingleTupleTableSlot(slot);
+         ereport(ERROR,

bt_verify_index_tuple_points_to_heap uses SnapshotAny with
table_tuple_fetch_row_version to distinguish "tuple doesn't exist" from
"tuple exists but is dead". However, bt_index_check only holds
AccessShareLock which compatible with VACUUM's ShareUpdateExclusiveLock.
A concurrent VACUUM Phase 1 can prune heap pages(ItemIdSetDead) between
Bloom filter probe and the heap lookup. Causing
table_tuple_fetch_row_version to return false for a legal
dead-but-now-pruned tuple and a false positive corruption report.

The table_tuple_satisfies_snapshot check does not help, it only runs
when the fetch succeeds(LP_NORMAL). Once VACUUM sets LP_DEAD,
heapam_fetch sees !ItemIdIsNormal(lp) and returns false before any
snapshot checks.

The reproduce should be: DELETE all rows form a big table, then launch
VACUUM and bt_index_check concurrently. A POC test script attached.


Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Fixes inconsistent behavior in vacuum when it processes multiple relations
Next
From: Bertrand Drouvot
Date:
Subject: Re: Adding locks statistics