From 23a1b8a8ce20acf770e86bd1fc03d272c816b332 Mon Sep 17 00:00:00 2001 From: Jakub Wartak Date: Wed, 8 Apr 2026 14:01:59 +0200 Subject: [PATCH v6 3/4] amcheck: detect lost heap segments based on index tuples bt_index_check() family of functions reads the whole btree index and builds a bloom filter along the way. Later it reads the heap table and for every tuple there it probes the filter. However when some segments of the main fork have been removed (e.g. by rogue action, filesystem corruption/fsck, missing full backup restore) this logic cannot reliably detect that the relation is 'short', as RelationGetNumberOfBlocks() stops counting on the last available heap segment, so the missing part of the heap is silently never probed against the index. Teach amcheck to verify during the btree scan that the currently processed leaf index tuple does not point to a heap block beyond the current relation size. This works with any bt_index_check() flags, does not require indexallkeysmatch, and reports a human-readable error instead of "could not open file" when the index tuple's TID falls into a lost segment. When the first segment of the heap's main fork is missing entirely, the beyond-EOF checks are skipped. That case is not silent data loss: every heap access already fails loudly with "could not open file". Historically bt_index_check() without heap-related options never touches the heap at all, and pg_amcheck relies on that when the user explicitly excludes a corrupt table (--exclude-table) while still checking its indexes. The TAP test requires a build with a tiny segment size (--with-segsize-blocks) to be able to simulate a lost segment on a small table, and is skipped otherwise. Author: Jakub Wartak Reviewed-by: Andrey Borodin Discussion: https://www.postgresql.org/message-id/flat/432626F9-65DF-4F0D-B345-26CFC3E2CFAC@yandex-team.ru --- contrib/amcheck/meson.build | 1 + .../t/008_verify_nbtree_lost_segments.pl | 126 ++++++++++++++++++ contrib/amcheck/verify_nbtree.c | 76 +++++++++++ 3 files changed, 203 insertions(+) create mode 100644 contrib/amcheck/t/008_verify_nbtree_lost_segments.pl diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build index 220b1ce1d59..28e3e17b447 100644 --- a/contrib/amcheck/meson.build +++ b/contrib/amcheck/meson.build @@ -52,6 +52,7 @@ tests += { 't/005_pitr.pl', 't/006_verify_gin.pl', 't/007_verify_nbtree_indexallkeysmatch.pl', + 't/008_verify_nbtree_lost_segments.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..5895bb4bafc --- /dev/null +++ b/contrib/amcheck/t/008_verify_nbtree_lost_segments.pl @@ -0,0 +1,126 @@ + +# 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"); + +($result, $stdout, $stderr) = $node->psql( + 'postgres', q(SELECT bt_index_check('bttest_unique_idx1', true, true, true);) +); +like( + $stderr, + qr/index line pointer in index "bttest_unique_idx1" points to missing page in table "missingsegs_test1"/, + 'detected corrupted segments for missingsegs_test1'); + +($result, $stdout, $stderr) = $node->psql( + 'postgres', q(SELECT bt_index_check('bttest_idx2', true, true, true);) +); +like( + $stderr, + qr/index line pointer in index "bttest_idx2" points to missing page in table "missingsegs_test2"/, + 'detected corrupted segments for missingsegs_test2'); + +$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 in accessible +sub corrupt_segment +{ + my ($relpath) = @_; + my $destrelpath = $relpath . ".BAK"; + + rename($relpath, $destrelpath) + or BAIL_OUT("rename failed: $!"); +} + diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 73e223e04fd..eff51c16390 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -154,6 +154,8 @@ typedef struct BtreeCheckState /* Reusable slot and executor state for FormIndexDatum() */ TupleTableSlot *iakm_slot; EState *iakm_estate; + /* Short heap segments verification */ + BlockNumber heapnblocks; } BtreeCheckState; /* @@ -453,6 +455,24 @@ 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. + * + * Those checks target silent data loss: when a higher-numbered segment + * of the main fork is missing or truncated, mdnblocks() returns a short + * size without any error. If instead the first segment is missing + * entirely, there is nothing silent to detect -- every heap access + * fails with "could not open file" -- so skip the beyond-EOF checks + * (by making any comparison against heapnblocks false) rather than + * fail an index-only check that otherwise would not touch the heap. + * This matters for pg_amcheck, which may check an index while its + * corrupt table was explicitly excluded from verification. + */ + 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; @@ -1637,6 +1657,62 @@ 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; + + if (BTreeTupleIsPosting(itup)) + nposting = BTreeTupleGetNPosting(itup); + + for (int 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)) { -- 2.50.1 (Apple Git-155)