From e8f3e116a8808139f0e3b7e93cdcad05894902e0 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sat, 17 Oct 2026 09:23:34 -0400 Subject: [PATCH v1 2/2] Add test for stale left link logged by btree page unlink. When VACUUM unlinks an internal btree page that sits above a half-dead leaf, _bt_unlink_halfdead_page() logs the leaf page's left sibling link so that replay can recreate the leaf; the UNLINK_PAGE record carries the leaf as a WILL_INIT buffer. Until the previous commit, that link was read while the leaf was pinned but not locked, and was not re-read when the leaf was locked again just before the record was written. If the leaf's left sibling split in that window, the leaf's btpo_prev advanced on the primary while the stale link was what got logged. Replay then built a leaf page whose left link skipped the new sibling, so a standby (and the primary after crash recovery) disagreed with the primary's own copy of the page. The new recovery test builds a small three-level index, pauses VACUUM at the nbtree-leave-page-half-dead injection point while it deletes an internal page and its only leaf child, splits the leaf's left sibling, and compares the leaf's left link on a streaming standby with the primary's. Without the previous commit, the standby's copy of the leaf still pointed to the pre-split sibling. --- src/test/recovery/Makefile | 1 + src/test/recovery/meson.build | 1 + .../t/061_nbtree_unlink_stale_leftlink.pl | 133 ++++++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 src/test/recovery/t/061_nbtree_unlink_stale_leftlink.pl diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile index 9c4102b6b..a91d014a0 100644 --- a/src/test/recovery/Makefile +++ b/src/test/recovery/Makefile @@ -10,6 +10,7 @@ #------------------------------------------------------------------------- EXTRA_INSTALL=contrib/pg_prewarm \ + contrib/pageinspect \ contrib/pg_stat_statements \ contrib/test_decoding \ src/test/modules/injection_points \ diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index b6ac9dd07..b9e4ee44c 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -69,6 +69,7 @@ tests += { 't/058_shutdown_crash_restart.pl', 't/059_remote_apply_status_interval.pl', 't/060_multixact_crashed_updater.pl', + 't/061_nbtree_unlink_stale_leftlink.pl', ], }, } diff --git a/src/test/recovery/t/061_nbtree_unlink_stale_leftlink.pl b/src/test/recovery/t/061_nbtree_unlink_stale_leftlink.pl new file mode 100644 index 000000000..3dfd97ffb --- /dev/null +++ b/src/test/recovery/t/061_nbtree_unlink_stale_leftlink.pl @@ -0,0 +1,133 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that redo of a btree page unlink recreates the half-dead leaf page with +# its current left sibling link. +# +# When VACUUM deletes a subtree (an internal page plus its only remaining leaf +# child), the UNLINK_PAGE record for the internal page also carries the leaf's +# sibling links, since redo rebuilds the leaf page from scratch. +# _bt_unlink_halfdead_page() used to read those links before it released its +# lock on the leaf, and did not read them again once it relocked the leaf to +# write the record. A concurrent split of the leaf's left sibling in that +# window advanced the leaf's left link on the primary, but the stale link was +# what got logged. Replay then built a leaf page whose left link skipped the +# new sibling, so a standby (or the primary after crash recovery) disagreed +# with the primary's own copy of the page. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1); +$primary->start; + +if (!$primary->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$primary->safe_psql('postgres', 'CREATE EXTENSION injection_points'); +$primary->safe_psql('postgres', 'CREATE EXTENSION pageinspect'); + +$primary->backup('backup'); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'backup', has_streaming => 1); +$standby->start; + +# A three-level index whose layout is fixed by its key width: each leaf page +# holds two keys, and each internal page up to three downlinks. (STORAGE +# PLAIN keeps the index from compressing the keys.) +# +# root +# / | \ +# [X] [P] [Z] +# / | \ / | \ / \ +# [A] [B] [C] [D] [E] [LEAF] [G] [H] +# 10 30 50 70 90 110 130 150 +# 20 40 60 80 100 120 140 160 +$primary->safe_psql( + 'postgres', q{ +CREATE TABLE t (i int, k text STORAGE PLAIN) WITH (autovacuum_enabled = off); +INSERT INTO t SELECT i, lpad(i::text, 2600, '0') FROM generate_series(10, 160, 10) i; +CREATE INDEX t_k ON t (k); +}); + +# P is the root's second downlink, and LEAF is P's last downlink +my $root = $primary->safe_psql('postgres', "SELECT root FROM bt_metap('t_k')"); +my $p = $primary->safe_psql('postgres', + "SELECT (ctid::text::point)[0]::int FROM bt_page_items('t_k', $root) WHERE itemoffset = 2" +); +my $leaf = $primary->safe_psql('postgres', + "SELECT (ctid::text::point)[0]::int FROM bt_page_items('t_k', $p) ORDER BY itemoffset DESC LIMIT 1" +); +my $leftlink = "SELECT btpo_prev FROM bt_page_stats('t_k', $leaf)"; + +# The rest of the test relies on that layout +my $leafkeys = $primary->safe_psql('postgres', + "SELECT string_agg(i::text, ' ' ORDER BY i) FROM t WHERE ctid IN (SELECT htid FROM bt_page_items('t_k', $leaf))" +); +is($leafkeys, '110 120', 'LEAF holds keys 110 and 120'); + +# Delete D and E, which leaves LEAF as P's only child, and then empty LEAF too. +# The next VACUUM deletes the {P, LEAF} subtree: it unlinks P first, then LEAF. +$primary->safe_psql( + 'postgres', q{ +DELETE FROM t WHERE i IN (70, 80, 90, 100); +VACUUM t; +DELETE FROM t WHERE i IN (110, 120); +}); + +# Run that VACUUM in the background, and stop it once it has read LEAF's +# sibling links and dropped its lock on LEAF, just before it unlinks P. +my $vacuum = $primary->background_psql('postgres'); +$vacuum->query_safe( + q{ +SELECT injection_points_set_local(); +SELECT injection_points_attach('nbtree-leave-page-half-dead', 'wait'); +}); +$vacuum->query_until(qr//, "VACUUM t;\n"); +$primary->wait_for_event('client backend', 'nbtree-leave-page-half-dead'); + +# Split C, LEAF's left sibling, which advances LEAF's left link to the new page +my $before = $primary->safe_psql('postgres', $leftlink); +$primary->safe_psql('postgres', + "INSERT INTO t VALUES (65, lpad('65', 2600, '0'))"); +my $after = $primary->safe_psql('postgres', $leftlink); +isnt($after, $before, "left sibling split while VACUUM held no lock on LEAF"); + +# Let VACUUM unlink P, whose UNLINK_PAGE record carries LEAF's left link, and +# stop it again before it unlinks LEAF itself +$primary->safe_psql('postgres', + "SELECT injection_points_wakeup('nbtree-leave-page-half-dead')"); +$primary->poll_query_until('postgres', + "SELECT type = 'D' FROM bt_page_stats('t_k', $p)") + or die "VACUUM did not unlink P"; +$primary->wait_for_event('client backend', 'nbtree-leave-page-half-dead'); + +# The record is not flushed before VACUUM commits, so force it out to the +# standby, which recreates LEAF when it replays the record +$primary->safe_psql('postgres', 'SELECT pg_switch_wal()'); +$primary->wait_for_catchup($standby); + +my $primary_leftlink = $primary->safe_psql('postgres', $leftlink); +my $standby_leftlink = $standby->safe_psql('postgres', $leftlink); +note + "LEAF left link: $before before the split, $after after; primary=$primary_leftlink standby=$standby_leftlink"; +is($standby_leftlink, $primary_leftlink, + 'replayed LEAF page has the same left link as on the primary'); + +# Let VACUUM finish +$primary->safe_psql('postgres', + "SELECT injection_points_wakeup('nbtree-leave-page-half-dead')"); +$vacuum->quit; + +done_testing(); -- 2.55.0