From 07d7539d4e21230551ff86ffa07b76a9de6a1e6b Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Sun, 8 Nov 2026 11:17:49 +0500 Subject: [PATCH v7 2/3] amcheck: detect dangling B-Tree references to heap storage Check leaf TIDs against heap size regardless of indexallkeysmatch. Refresh the size before reporting corruption to allow concurrent extension. Skip the check when the main fork is absent, so indexes can still be verified independently of the missing heap fork. On a reverse-key lookup that finds no visible tuple, distinguish an unused or out-of-range heap slot from a dead tuple only with ShareLock. This blocks VACUUM from recycling an indexed slot, while allowing opportunistic pruning of HOT chains. AccessShareLock cannot make that distinction. Combine the beyond-EOF checks in one place. Add tests of dangling slots and missing segments, including calls with indexallkeysmatch disabled. Author: Andrey Borodin Author: Jakub Wartak Reviewed-by: Zsolt Parragi Reviewed-by: Wenbo Lin Discussion: https://postgr.es/m/432626F9-65DF-4F0D-B345-26CFC3E2CFAC@yandex-team.ru --- contrib/amcheck/meson.build | 2 + .../t/008_verify_nbtree_lost_segments.pl | 121 +++++++++++++ .../amcheck/t/009_verify_nbtree_dangling.pl | 82 +++++++++ contrib/amcheck/verify_nbtree.c | 166 +++++++++++++++++- 4 files changed, 369 insertions(+), 2 deletions(-) create mode 100644 contrib/amcheck/t/008_verify_nbtree_lost_segments.pl create mode 100644 contrib/amcheck/t/009_verify_nbtree_dangling.pl diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build index 220b1ce1d59..68dcbacb226 100644 --- a/contrib/amcheck/meson.build +++ b/contrib/amcheck/meson.build @@ -52,6 +52,8 @@ tests += { 't/005_pitr.pl', 't/006_verify_gin.pl', 't/007_verify_nbtree_indexallkeysmatch.pl', + 't/008_verify_nbtree_lost_segments.pl', + 't/009_verify_nbtree_dangling.pl', ], }, } diff --git a/contrib/amcheck/t/008_verify_nbtree_lost_segments.pl b/contrib/amcheck/t/008_verify_nbtree_lost_segments.pl new file mode 100644 index 00000000000..16b44b05041 --- /dev/null +++ b/contrib/amcheck/t/008_verify_nbtree_lost_segments.pl @@ -0,0 +1,121 @@ + +# Copyright (c) 2023-2026, PostgreSQL Global Development Group + +# This regression test checks the behavior of the btree validation in the +# presence of missing relation segments. +# +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('test'); +$node->init; +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->start; + +# Create two tables, one with unique index and another to test +# posting list (btree duplicates). +$node->safe_psql( + 'postgres', q( + CREATE EXTENSION amcheck; + CREATE TABLE missingsegs_test1 AS + SELECT * FROM generate_series(1, 3000) id; + CREATE TABLE missingsegs_test2 AS + SELECT 10 AS id FROM generate_series(1, 3000); + CREATE UNIQUE INDEX bttest_unique_idx1 ON missingsegs_test1 (id); + CREATE INDEX bttest_idx2 ON missingsegs_test2 (id); +)); + +my ($result, $stdout, $stderr); + +# The test tables are small, so a second relation segment only exists when +# the server was built with a tiny segment size (--with-segsize-blocks). +# There is no way to simulate a lost segment otherwise; skip in that case. +my $segpath1 = relation_filepath('missingsegs_test1') . ".1"; +my $segpath2 = relation_filepath('missingsegs_test2') . ".1"; +if (!-e $segpath1 || !-e $segpath2) +{ + plan skip_all => + 'test requires small relation segment size (--with-segsize-blocks)'; +} + +# We have not yet broken the index, so we should get no corruption +$result = $node->safe_psql( + 'postgres', q( + SELECT bt_index_check('bttest_unique_idx1', true, true, true); +)); +is($result, '', 'run amcheck on non-broken bttest_unique_idx1'); + +$result = $node->safe_psql( + 'postgres', q( + SELECT bt_index_check('bttest_idx2', true, true, true); +)); +is($result, '', 'run amcheck on non-broken bttest_idx2'); + +# Break the relations, simulating rogue action or just fsck moving files +# into the /lost+found. +my $relpath1 = relation_filepath('missingsegs_test1'); +my $relpath2 = relation_filepath('missingsegs_test2'); +$node->stop; +corrupt_segment($relpath1 . ".1"); +corrupt_segment($relpath2 . ".1"); +$node->start; + +$result = $node->safe_psql( + 'postgres', q( + SET enable_indexscan TO off; + SET enable_indexonlyscan TO off; + SELECT count(id) FROM missingsegs_test1; +)); +cmp_ok(3000, '>', $result, + "ensure there is missing data on missingsegs_test1"); + +$result = $node->safe_psql( + 'postgres', q( + SET enable_indexscan TO off; + SET enable_indexonlyscan TO off; + SELECT count(id) FROM missingsegs_test2; +)); +cmp_ok(3000, '>', $result, + "ensure there is missing data on missingsegs_test2"); + +# Beyond-EOF checks must not depend on indexallkeysmatch being enabled. +for my $index ('bttest_unique_idx1', 'bttest_idx2') +{ + for my $options ('', ', true, true, true') + { + ($result, $stdout, $stderr) = $node->psql('postgres', + qq(SELECT bt_index_check('$index'$options))); + is($result, 3, "missing segment rejected by bt_index_check('$index'$options)"); + like($stderr, + qr/index line pointer in index "$index" points to missing page in table/, + "missing page identified by bt_index_check('$index'$options)"); + } +} + +$node->stop; +done_testing(); + +# Returns the filesystem path for the named relation. +sub relation_filepath +{ + my ($relname) = @_; + + my $pgdata = $node->data_dir; + my $rel = $node->safe_psql('postgres', + qq(SELECT pg_relation_filepath('$relname'))); + die "path not found for relation $relname" unless defined $rel; + return "$pgdata/$rel"; +} + +# Rename segment so that it is inaccessible. +sub corrupt_segment +{ + my ($relpath) = @_; + my $destrelpath = $relpath . ".BAK"; + + rename($relpath, $destrelpath) + or BAIL_OUT("rename failed: $!"); +} diff --git a/contrib/amcheck/t/009_verify_nbtree_dangling.pl b/contrib/amcheck/t/009_verify_nbtree_dangling.pl new file mode 100644 index 00000000000..a285cab839c --- /dev/null +++ b/contrib/amcheck/t/009_verify_nbtree_dangling.pl @@ -0,0 +1,82 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# An index-referenced heap slot may become LP_DEAD through pruning, but +# cannot become LP_UNUSED while bt_index_parent_check holds ShareLock. +# bt_index_check cannot make the same distinction from concurrent VACUUM. +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('test'); +$node->init(no_data_checksums => 1); +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->start; +$node->safe_psql('postgres', q( + CREATE EXTENSION amcheck; + CREATE EXTENSION pageinspect; + CREATE TABLE dangling_unused (id int); + INSERT INTO dangling_unused VALUES (1); + CREATE INDEX dangling_unused_idx ON dangling_unused (id); + CREATE TABLE dangling_offset (id int); + INSERT INTO dangling_offset VALUES (1); + CREATE INDEX dangling_offset_idx ON dangling_offset (id); + CREATE TABLE dangling_dead (id int); + INSERT INTO dangling_dead VALUES (1); + CREATE INDEX dangling_dead_idx ON dangling_dead (id); + DELETE FROM dangling_dead; +)); +$node->safe_psql('postgres', + 'VACUUM (INDEX_CLEANUP OFF, TRUNCATE OFF) dangling_dead'); +is($node->safe_psql('postgres', q( + SELECT lp_flags FROM heap_page_items(get_raw_page('dangling_dead', 0)) + WHERE lp = 1; +)), '3', 'pruning left an LP_DEAD slot awaiting index cleanup'); +is($node->safe_psql('postgres', q( + SELECT bt_index_parent_check('dangling_dead_idx', false, false, false, true); +)), '', 'LP_DEAD is not a dangling index entry'); + +my %paths; +for my $table ('dangling_unused', 'dangling_offset') +{ + $paths{$table} = $node->data_dir . '/' . $node->safe_psql('postgres', + qq(SELECT pg_relation_filepath('$table'))); +} +$node->stop; + +# Zero the first ItemIdData (just after the 24-byte PageHeaderData) to +# turn the indexed tuple into LP_UNUSED. Separately, move pd_lower to +# the end of the page header so that offset 1 is beyond the ItemId array. +overwrite($paths{dangling_unused}, 24, pack('L', 0)); +overwrite($paths{dangling_offset}, 12, pack('S', 24)); +$node->start; + +for my $case ( + ['dangling_unused', qr/points to unused heap slot/], + ['dangling_offset', qr/with out-of-range offset/]) +{ + my ($table, $error) = @$case; + is($node->safe_psql('postgres', + qq(SELECT bt_index_check('${table}_idx', false, false, true))), + '', "bt_index_check skips $table without ShareLock"); + my ($ret, $out, $err) = $node->psql('postgres', + qq(SELECT bt_index_parent_check('${table}_idx', false, false, false, true))); + is($ret, 3, "bt_index_parent_check rejects $table"); + like($err, $error, "bt_index_parent_check identifies $table"); +} + +$node->stop; +done_testing(); + +sub overwrite +{ + my ($path, $offset, $bytes) = @_; + open(my $fh, '+<', $path) or BAIL_OUT("open failed: $!"); + binmode $fh; + defined(sysseek($fh, $offset, 0)) or BAIL_OUT("sysseek failed: $!"); + syswrite($fh, $bytes) == length($bytes) + or BAIL_OUT("syswrite failed: $!"); + close($fh) or BAIL_OUT("close failed: $!"); +} diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 94e43a23e52..59516bde6e6 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -151,6 +151,8 @@ typedef struct BtreeCheckState /* Reusable slot and executor state for FormIndexDatum() */ TupleTableSlot *iakm_slot; EState *iakm_estate; + /* Cached heap size for unconditional beyond-EOF verification */ + BlockNumber heapnblocks; } BtreeCheckState; /* @@ -239,6 +241,10 @@ static void bt_verify_index_tuple_points_to_heap(BtreeCheckState *state, IndexTuple itup, BlockNumber targetblock, OffsetNumber offset); +static void bt_report_dangling_index_entry(BtreeCheckState *state, + ItemPointer tid, + BlockNumber targetblock, + OffsetNumber offset); static IndexTuple bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup); static inline IndexTuple bt_posting_plain_tuple(IndexTuple itup, int n); @@ -432,6 +438,18 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace, state->heaprel = heaprel; state->heapkeyspace = heapkeyspace; state->readonly = readonly; + + /* + * Remember the current size of the heap for beyond-EOF TID checks. When + * the main fork is missing entirely, skip those checks (by making any + * comparison against heapnblocks false): the user may have deliberately + * excluded the corrupt table from table-level verification, which is what + * reports a missing main fork. + */ + if (smgrexists(RelationGetSmgr(state->heaprel), MAIN_FORKNUM)) + state->heapnblocks = RelationGetNumberOfBlocks(state->heaprel); + else + state->heapnblocks = InvalidBlockNumber; state->heapallindexed = heapallindexed; state->indexallkeysmatch = indexallkeysmatch; state->rootdescend = rootdescend; @@ -1614,6 +1632,63 @@ bt_target_page_check(BtreeCheckState *state) } } + /* Check that leaf page tuples do not point beyond the end of heap */ + if (P_ISLEAF(topaque) && !ItemIdIsDead(itemid)) + { + int nposting = 1; + int i; + + if (BTreeTupleIsPosting(itup)) + nposting = BTreeTupleGetNPosting(itup); + + for (i = 0; i < nposting; i++) + { + ItemPointer htid; + BlockNumber heapblk; + OffsetNumber heapoff; + + if (nposting > 1) + htid = BTreeTupleGetPostingN(itup, i); + else + htid = BTreeTupleGetPointsToTID(itup); + + heapblk = ItemPointerGetBlockNumber(htid); + heapoff = ItemPointerGetOffsetNumber(htid); + + /* + * Does heapblk go beyond RelationGetNumberOfBlocks(), + * potentially indicating a missing relation segment? + */ + if (state->heapnblocks != InvalidBlockNumber && + heapblk >= state->heapnblocks) + { + /* + * The relation may have been extended concurrently; + * refresh the cached value before reporting corruption. + */ + state->heapnblocks = RelationGetNumberOfBlocks(state->heaprel); + if (heapblk >= state->heapnblocks) + { + char *postingoff = ""; + + if (nposting > 1) + postingoff = psprintf(" posting list offset=%d", i); + + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index line pointer in index \"%s\" points to missing page in table \"%s\"", + RelationGetRelationName(state->rel), + RelationGetRelationName(state->heaprel)), + errdetail_internal("Index tid=(%u,%u)%s points to heap tid=(%u,%u) but heap has only %u blocks.", + state->targetblock, offset, postingoff, + heapblk, heapoff, + state->heapnblocks), + errhint("This can be caused by a lost relation segment (missing or removed file)."))); + } + } + } + } + /* Verify each index tuple points to heap tuple with same key */ if (state->indexallkeysmatch && P_ISLEAF(topaque) && !ItemIdIsDead(itemid)) { @@ -2972,8 +3047,10 @@ bt_heap_fingerprint_callback(Relation index, ItemPointer tid, Datum *values, * chains, resolving the TID of the visible tuple version, which we then * load into a slot with table_tuple_fetch_row_version(). A "not found" * result means the tuple is dead or was concurrently pruned; we skip it - * exactly as an index scan would. Here we verify that the visible - * heap tuple matches the index key. + * exactly as an index scan would. Detecting index entries that point to + * nonexistent heap slots is a separate class of corruption, handled + * elsewhere; what we verify here is that the visible heap tuple matches the + * index key. */ static void bt_verify_index_tuple_points_to_heap(BtreeCheckState *state, IndexTuple itup, @@ -3022,7 +3099,18 @@ bt_verify_index_tuple_points_to_heap(BtreeCheckState *state, IndexTuple itup, */ visibletid = *tid; if (!table_fetch_tid(state->heaprel, &visibletid, state->snapshot, NULL)) + { + /* + * No visible tuple at this TID. Normally that just means the + * index entry is for a dead (or concurrently pruned) heap tuple + * and there is nothing to compare, but it can also mean the index + * entry is dangling: pointing to a heap slot that holds no tuple + * at all. Try to tell these cases apart when it is safe to do + * so. + */ + bt_report_dangling_index_entry(state, tid, targetblock, offset); return; + } if (!table_tuple_fetch_row_version(state->heaprel, &visibletid, state->snapshot, slot)) @@ -3061,6 +3149,80 @@ bt_verify_index_tuple_points_to_heap(BtreeCheckState *state, IndexTuple itup, } } +/* + * Try to detect a dangling index entry: an index tuple whose TID points to + * a heap slot that holds no tuple at all (as opposed to a dead one awaiting + * VACUUM). Called when an MVCC fetch of the TID found no visible tuple. + * + * References beyond the end of the heap have already been checked in + * bt_target_page_check(), independently of indexallkeysmatch. + * + * Distinguishing other dangling cases (out-of-range offset, LP_UNUSED slot) + * from a concurrently pruned tuple requires that the heap slot cannot + * change under us. Under ShareLock (bt_index_parent_check) concurrent + * VACUUM is blocked, so an index-referenced slot cannot become LP_UNUSED + * and the line pointer array cannot be truncated: opportunistic pruning may + * mark slots LP_DEAD or redirect them, but it only marks HOT-chain member + * slots LP_UNUSED, and those are never referenced from indexes. Under + * AccessShareLock (bt_index_check) concurrent VACUUM may reclaim the slot + * between our Bloom filter probe and the fetch, so we must not report + * anything. + * + * Direct line pointer inspection requires heap page layout, so it is + * skipped for non-heap table AMs. + */ +static void +bt_report_dangling_index_entry(BtreeCheckState *state, ItemPointer tid, + BlockNumber targetblock, OffsetNumber offset) +{ + BlockNumber blkno = ItemPointerGetBlockNumber(tid); + OffsetNumber offnum = ItemPointerGetOffsetNumber(tid); + Buffer buffer; + Page page; + ItemId lp; + + /* + * Without ShareLock a missing slot is indistinguishable from a + * concurrently pruned tuple; report nothing. + */ + if (!state->readonly) + return; + + /* Line pointer inspection below assumes heap page layout */ + if (state->heaprel->rd_rel->relam != HEAP_TABLE_AM_OID) + return; + + buffer = ReadBufferExtended(state->heaprel, MAIN_FORKNUM, blkno, + RBM_NORMAL, state->checkstrategy); + LockBuffer(buffer, BUFFER_LOCK_SHARE); + page = BufferGetPage(buffer); + + if (offnum > PageGetMaxOffsetNumber(page)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index tuple in index \"%s\" points to heap tuple in table \"%s\" with out-of-range offset", + RelationGetRelationName(state->rel), + RelationGetRelationName(state->heaprel)), + errdetail_internal("Index tid=(%u,%u) points to heap tid=(%u,%u), but page has only %u line pointers.", + targetblock, offset, + blkno, offnum, + (unsigned) PageGetMaxOffsetNumber(page)))); + + lp = PageGetItemId(page, offnum); + if (!ItemIdIsUsed(lp)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index tuple in index \"%s\" points to unused heap slot in table \"%s\"", + RelationGetRelationName(state->rel), + RelationGetRelationName(state->heaprel)), + errdetail_internal("Index tid=(%u,%u) points to heap tid=(%u,%u) marked LP_UNUSED.", + targetblock, offset, + blkno, offnum))); + + /* LP_DEAD, LP_REDIRECT or a dead LP_NORMAL tuple: not corruption */ + UnlockReleaseBuffer(buffer); +} + /* * Normalize an index tuple for fingerprinting. * -- 2.50.1 (Apple Git-155)