From e1621ebc19da29dc2d6ecf41eb2422014fb382cf Mon Sep 17 00:00:00 2001 From: Salma Date: Tue, 15 Sep 2026 12:55:25 +0300 Subject: [PATCH v3 3/3] amcheck: Add verification for B-tree page merges Extend verify_nbtree to validate structural invariants for merged and tombstone leaf pages. --- contrib/amcheck/Makefile | 2 +- contrib/amcheck/t/007_btree_merge_wal.pl | 227 +++++++++++++++++++++++ contrib/amcheck/verify_nbtree.c | 162 +++++++++++++++- 3 files changed, 383 insertions(+), 8 deletions(-) create mode 100644 contrib/amcheck/t/007_btree_merge_wal.pl diff --git a/contrib/amcheck/Makefile b/contrib/amcheck/Makefile index 1b7a63cbaa4..0321cc9f094 100644 --- a/contrib/amcheck/Makefile +++ b/contrib/amcheck/Makefile @@ -15,7 +15,7 @@ PGFILEDESC = "amcheck - function for verifying relation integrity" REGRESS = check check_btree check_gin check_heap -EXTRA_INSTALL = contrib/pg_walinspect +EXTRA_INSTALL = contrib/pg_walinspect contrib/pageinspect TAP_TESTS = 1 ifdef USE_PGXS diff --git a/contrib/amcheck/t/007_btree_merge_wal.pl b/contrib/amcheck/t/007_btree_merge_wal.pl new file mode 100644 index 00000000000..13c5482ee0d --- /dev/null +++ b/contrib/amcheck/t/007_btree_merge_wal.pl @@ -0,0 +1,227 @@ + +# Copyright (c) 2024-2026, PostgreSQL Global Development Group + +# +# Tests for B-tree page merge WAL logging. +# +# Tests that a merged destination page (BTP_MERGED) that is subsequently +# deduplicated and split correctly retains the merged-away block number +# through each WAL redo path, and that VACUUM-driven merge cleanup properly +# resolves recovery conflicts on a Hot Standby. +# +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Build a table with enough sparse pages for bt_merge() to find candidates. +# Autovacuum is disabled so VACUUM runs only when the test asks for it. +my $setup_sql = q{ + CREATE TABLE merge_test (id int); + ALTER TABLE merge_test SET (autovacuum_enabled = false); + INSERT INTO merge_test SELECT i FROM generate_series(1, 10000) i; + CREATE INDEX merge_test_idx ON merge_test(id) WITH (deduplicate_items = on); + DELETE FROM merge_test WHERE id % 20 != 0; + VACUUM merge_test; +}; + +my $check_sql = q{SELECT bt_index_check('merge_test_idx', true)}; + +# Count leaf pages that carry BTP_MERGED (flag bit 512). +my $merged_page_count_sql = q{ + SELECT count(*) + FROM generate_series( + 1, + pg_relation_size('merge_test_idx') / + current_setting('block_size')::int - 1) AS b(blkno) + CROSS JOIN LATERAL bt_page_stats('merge_test_idx', b.blkno) AS stats + WHERE stats.type = 'l' AND (stats.btpo_flags & 512) <> 0 +}; + +# +# Run a merge followed by a deduplication pass and a page split, bracketing +# each workload with LSN snapshots so the caller can inspect WAL ranges. +# Also asserts that the split caused a new BTP_MERGED page (proving the split +# redo path propagates the flag to the right half). +# +sub merge_and_stress +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my ($node, $name) = @_; + + my $merges = $node->safe_psql('postgres', + q{SELECT merges_performed + FROM bt_merge('merge_test_idx', 10.0, 90.0, 10)}); + isnt($merges, '0', "$name: bt_merge() performed at least one merge"); + + my $merged_before = $node->safe_psql('postgres', $merged_page_count_sql); + + my $dedup_start = $node->safe_psql('postgres', + 'SELECT pg_current_wal_lsn()'); + + # Insert many copies of a key that lands on a merged page to force a + # DEDUP record on that page. + $node->safe_psql('postgres', + q{INSERT INTO merge_test SELECT 20 FROM generate_series(1, 500)}); + + my $dedup_end = $node->safe_psql('postgres', 'SELECT pg_current_wal_lsn()'); + + my $split_start = $node->safe_psql('postgres', + 'SELECT pg_current_wal_lsn()'); + + # Re-insert the full key range to fill every merged page and force a split. + $node->safe_psql('postgres', + q{INSERT INTO merge_test SELECT i FROM generate_series(1, 10000) i}); + + my $split_end = $node->safe_psql('postgres', 'SELECT pg_current_wal_lsn()'); + + my $merged_after = $node->safe_psql('postgres', $merged_page_count_sql); + cmp_ok($merged_after, '>', $merged_before, + "$name: split produced a new BTP_MERGED page"); + + return ($dedup_start, $dedup_end, $split_start, $split_end); +} + +sub waldump_range +{ + my ($node, $start_lsn, $end_lsn) = @_; + my $waldir = $node->data_dir . '/pg_wal'; + + return qx{pg_waldump -p $waldir -s $start_lsn -e $end_lsn 2>/dev/null}; +} + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init(allows_streaming => 1); +$node->start; + +# Install extensions once; $setup_sql is called repeatedly and must not +# repeat CREATE EXTENSION. +$node->safe_psql('postgres', + 'CREATE EXTENSION amcheck; CREATE EXTENSION pageinspect'); + +$node->safe_psql('postgres', $setup_sql); + + +# +# Verify that bt_merge(), the subsequent dedup pass, and the forced page split +# each emit the expected WAL record types. +# + +my ($dedup_start, $dedup_end, $split_start, $split_end) = + merge_and_stress($node, 'WAL generation'); +$node->safe_psql('postgres', 'CHECKPOINT'); + +like( + waldump_range($node, $dedup_start, $dedup_end), + qr/desc: DEDUP/, + 'pg_waldump shows a DEDUP record after inserting into a merged page'); + +like( + waldump_range($node, $split_start, $split_end), + qr/desc: SPLIT_[LR]/, + 'pg_waldump shows a SPLIT record after filling merged pages'); + +$node->safe_psql('postgres', $check_sql); + +# +# With wal_consistency_checking = 'btree' and full_page_writes = off, the +# server re-applies every btree WAL record to a scratch copy of the page and +# compares the result byte-for-byte. Any difference panics the server. +# Surviving the workload proves the DEDUP and SPLIT redo paths preserve the +# MA block number stored in pd_prune_xid. +# + +$node->append_conf('postgresql.conf', + "wal_consistency_checking = 'btree'\nfull_page_writes = off"); +$node->restart; + +$node->safe_psql('postgres', 'DROP TABLE merge_test CASCADE'); +$node->safe_psql('postgres', $setup_sql); +merge_and_stress($node, 'WAL consistency checking'); + +# Server is still up — redo matched the write side for every record. +$node->safe_psql('postgres', $check_sql); + +# +# stop('immediate') is equivalent to SIGKILL: dirty pages are lost. Recovery +# must redo the MERGE, DEDUP, and SPLIT records to reconstruct the index. +# + +$node->safe_psql('postgres', 'DROP TABLE merge_test CASCADE'); +$node->safe_psql('postgres', $setup_sql); +merge_and_stress($node, 'crash recovery'); + +$node->stop('immediate'); +$node->start; + +$node->safe_psql('postgres', $check_sql); +pass('bt_index_check passes after crash recovery with merged pages'); + +# +# Streaming replica: with full_page_writes still off, the standby must +# reconstruct every modified page from deltas alone. We also verify that +# VACUUM-driven merge cleanup triggers a snapshot recovery conflict for a +# standby transaction whose snapshot predates the merge's safemergexid. +# + +$node->safe_psql('postgres', 'DROP TABLE merge_test CASCADE'); +$node->safe_psql('postgres', $setup_sql); + +$node->backup('merge_backup'); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($node, 'merge_backup', has_streaming => 1); +$standby->append_conf('postgresql.conf', + "max_standby_streaming_delay = '50ms'"); +$standby->start; + +# Open a repeatable-read transaction on the standby before the merge runs so +# its snapshot predates the merge's safemergexid. A plain SELECT avoids +# leaving a buffer pin, which would produce a pin conflict rather than the +# snapshot conflict we intend to test. +my $standby_psql = + $standby->background_psql('postgres', on_error_stop => 0); +my $result = $standby_psql->query_safe(q{ + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; +}); +like($result, qr/^1$/m, 'standby snapshot established before merge'); + +merge_and_stress($node, 'streaming replication'); +$node->wait_for_replay_catchup($standby); + +# bt_index_check on the standby exercises the delta redo paths for MERGE, +# DEDUP, and SPLIT — the paths most likely to corrupt the MA block number. +$standby->safe_psql('postgres', $check_sql); +pass( + 'bt_index_check passes on standby after replaying merge WAL (full_page_writes off)' +); + +# VACUUM clears the BTP_MERGED flags and writes a CLEAR_MERGE_FLAG record +# carrying safemergexid. On replay, ResolveRecoveryConflictWithSnapshotFullXid +# cancels the standby transaction opened above. +my $log_offset = -s $standby->logfile; +$node->safe_psql('postgres', 'VACUUM merge_test'); +$node->wait_for_replay_catchup($standby); + +my $new_offset = $standby->wait_for_log( + qr/User query might have needed to see row versions that must be removed/, + $log_offset); +cmp_ok($new_offset, '>', $log_offset, + 'merge cleanup WAL canceled the old standby snapshot'); + +is($standby->safe_psql('postgres', q{ + SELECT confl_snapshot + FROM pg_stat_database_conflicts + WHERE datname = current_database() +}), '1', 'pg_stat_database_conflicts records one snapshot conflict'); + +$standby_psql->quit; +$standby->safe_psql('postgres', $check_sql); +pass('bt_index_check passes on standby after merge cleanup replay'); + +$standby->stop; +$node->stop; + +done_testing(); diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 3ef2d66f826..2553ee2b327 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -238,6 +238,8 @@ static ItemId PageGetItemIdCareful(BtreeCheckState *state, BlockNumber block, static inline ItemPointer BTreeTupleGetHeapTIDCareful(BtreeCheckState *state, IndexTuple itup, bool nonpivot); static inline ItemPointer BTreeTupleGetPointsToTID(IndexTuple itup); +static void bt_check_ma_page(BtreeCheckState *state); +static void bt_check_m_page(BtreeCheckState *state); /* * bt_index_check(index regclass, heapallindexed boolean, checkunique boolean) @@ -661,7 +663,7 @@ bt_check_level_from_leftmost(BtreeCheckState *state, BtreeLevel level) opaque = BTPageGetOpaque(state->target); - if (P_IGNORE(opaque)) + if (P_IGNORE(opaque) || P_ISMERGEDAWAY(opaque)) { /* * Since there cannot be a concurrent VACUUM operation in readonly @@ -682,6 +684,9 @@ bt_check_level_from_leftmost(BtreeCheckState *state, BtreeLevel level) errdetail_internal("Block=%u left block=%u left link from block=%u.", current, leftcurrent, opaque->btpo_prev))); + if (P_ISMERGEDAWAY(opaque)) + bt_check_ma_page(state); + if (P_RIGHTMOST(opaque)) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), @@ -818,7 +823,7 @@ nextpage: * splits wasn't investigated yet. Thankfully we only need low key * for readonly verification and concurrent splits won't happen. */ - if (state->readonly && !P_RIGHTMOST(opaque)) + if (state->readonly && !P_RIGHTMOST(opaque) && !P_ISMERGEDAWAY(opaque)) { IndexTuple itup; ItemId itemid; @@ -1281,6 +1286,9 @@ bt_target_page_check(BtreeCheckState *state) } } + if (P_ISMERGED(topaque)) + bt_check_m_page(state); + /* * Loop over page items, starting from first non-highkey item, not high * key (if any). Most tests are not performed for the "negative infinity" @@ -1740,7 +1748,7 @@ bt_target_page_check(BtreeCheckState *state) /* * All !readonly checks now performed; just return */ - if (P_IGNORE(topaque)) + if (P_IGNORE(topaque) || P_ISMERGEDAWAY(topaque)) return; } @@ -1790,7 +1798,7 @@ bt_target_page_check(BtreeCheckState *state) rightblock_number); topaque = BTPageGetOpaque(rightpage); - if (P_IGNORE(topaque)) + if (P_IGNORE(topaque) || P_ISMERGEDAWAY(topaque)) { pfree(rightpage); break; @@ -1912,7 +1920,7 @@ bt_right_page_check_scankey(BtreeCheckState *state, OffsetNumber *rightfirstoffs rightpage = palloc_btree_page(state, targetnext); opaque = BTPageGetOpaque(rightpage); - if (!P_IGNORE(opaque) || P_RIGHTMOST(opaque)) + if ((!P_IGNORE(opaque) && !P_ISMERGEDAWAY(opaque)) || P_RIGHTMOST(opaque)) break; /* @@ -2258,7 +2266,8 @@ bt_child_highkey_check(BtreeCheckState *state, * If we visit page with high key, check that it is equal to the * target key next to corresponding downlink. */ - if (!rightsplit && !P_RIGHTMOST(opaque) && !P_ISHALFDEAD(opaque)) + if (!rightsplit && !P_RIGHTMOST(opaque) && !P_ISHALFDEAD(opaque) && + !P_ISMERGEDAWAY(opaque)) { BTPageOpaque topaque; IndexTuple highkey; @@ -2497,6 +2506,22 @@ bt_child_check(BtreeCheckState *state, BTScanInsert targetkey, state->targetblock, childblock, LSN_FORMAT_ARGS(state->targetlsn)))); + /* + * A merged-away (MA) page is a tombstone that should have had its parent + * downlink redirected to the merged destination page (R) during the merge + * operation. In readonly mode, no concurrent merge can be in progress, so + * finding a downlink to an MA page indicates corruption. + */ + if (P_ISMERGEDAWAY(copaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("downlink to merged-away page found in index \"%s\"", + RelationGetRelationName(state->rel)), + errdetail_internal("Parent block=%u child block=%u parent page lsn=%X/%08X.", + state->targetblock, childblock, + LSN_FORMAT_ARGS(state->targetlsn)))); + + for (offset = P_FIRSTDATAKEY(copaque); offset <= maxoffset; offset = OffsetNumberNext(offset)) @@ -2611,6 +2636,13 @@ bt_downlink_missing_check(BtreeCheckState *state, bool rightsplit, return; } + /* + * A merged-away leaf page intentionally lacks a parent downlink, as it + * was unlinked during the custom bt_merge operation. + */ + if (P_ISMERGEDAWAY(opaque)) + return; + /* * Page under check is probably the "top parent" of a multi-level page * deletion. We'll need to descend the subtree to make sure that @@ -3435,7 +3467,7 @@ palloc_btree_page(BtreeCheckState *state, BlockNumber blocknum) errmsg_internal("internal page block %u in index \"%s\" has garbage items", blocknum, RelationGetRelationName(state->rel)))); - if (P_HAS_FULLXID(opaque) && !P_ISDELETED(opaque)) + if (P_HAS_FULLXID(opaque) && (!P_ISDELETED(opaque) && !P_ISMERGEDAWAY(opaque))) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), errmsg_internal("full transaction id page flag appears in non-deleted block %u in index \"%s\"", @@ -3591,3 +3623,119 @@ BTreeTupleGetPointsToTID(IndexTuple itup) /* Pivot tuple returns TID with downlink block (heapkeyspace variant) */ return &itup->t_tid; } + + +static void +bt_check_ma_page(BtreeCheckState *state) +{ + Page page = state->target; + BlockNumber block = state->targetblock; + BTPageOpaque opaque = BTPageGetOpaque(page); + + + if (!P_ISLEAF(opaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged-away block %u is not a leaf page in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + if (P_ISROOT(opaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged-away block %u cannot be root page in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + + if (P_ISMERGED(opaque) || P_ISHALFDEAD(opaque) || P_ISDELETED(opaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged-away block %u has conflicting page flags in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + if (opaque->btpo_next == P_NONE) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged-away block %u lacks right sibling in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + if (!P_HAS_FULLXID(opaque) || + !FullTransactionIdIsValid(BTMergedAwayGetSafeXid(page))) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged-away block %u has invalid safemergexid in index \"%s\"", + block, RelationGetRelationName(state->rel)))); +} + +static void +bt_check_m_page(BtreeCheckState *state) +{ + Page page = state->target; + BlockNumber block = state->targetblock; + BTPageOpaque opaque = BTPageGetOpaque(page); + + BlockNumber ma_blkno; + Page ma_page; + BTPageOpaque ma_opaque; + + if (!P_ISLEAF(opaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged page %u is not a leaf page in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + if (P_ISMERGEDAWAY(opaque)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged page %u cannot also be a merged-away tombstone in index \"%s\"", + block, RelationGetRelationName(state->rel)))); + + ma_blkno = BTMergedPageGetMABlkno(page); + + if (!BlockNumberIsValid(ma_blkno) || + ma_blkno == BTREE_METAPAGE || + ma_blkno == block) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged page %u has invalid MA block number %u in index \"%s\"", + block, ma_blkno, RelationGetRelationName(state->rel)))); + + ma_page = palloc_btree_page(state, ma_blkno); + ma_opaque = BTPageGetOpaque(ma_page); + + if (state->readonly && !P_ISMERGEDAWAY(ma_opaque)) + { + pfree(ma_page); + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("merged page %u points to MA block %u which is not a merged-away tombstone in index \"%s\"", + block, ma_blkno, RelationGetRelationName(state->rel)))); + } + + pfree(ma_page); + + /* + * Verify adjacent merge group consistency: In readonly mode (where no + * concurrent VACUUM/merge can occur), if the right sibling is also an M + * page, it must belong to the same merge group (i.e. name the exact same + * MA block number). + */ + if (state->readonly && opaque->btpo_next != P_NONE) + { + Page next_page = palloc_btree_page(state, opaque->btpo_next); + BTPageOpaque next_opaque = BTPageGetOpaque(next_page); + + if (P_ISMERGED(next_opaque) && + BTMergedPageGetMABlkno(next_page) != ma_blkno) + { + BlockNumber next_ma = BTMergedPageGetMABlkno(next_page); + + pfree(next_page); + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("adjacent merged pages %u and %u have inconsistent MA block numbers (%u vs %u) in index \"%s\"", + block, opaque->btpo_next, ma_blkno, next_ma, + RelationGetRelationName(state->rel)))); + } + pfree(next_page); + } +} -- 2.43.0