From 35a214c5682b482b807b9aa86056d837def2c0d5 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Wed, 19 Aug 2026 18:01:40 -0400 Subject: [PATCH 2/2] Add repro: GIN page reuse raises no recovery conflict on a standby. GIN decides a deleted page is reusable from the primary's horizon alone and writes no WAL when it hands the page out, so nothing cancels a standby scan that still holds a reference to it. nbtree and GiST both log a page reuse record for this. The test added by this commit shows how this oversight can lead to wrong answers on standbys. Co-Authored-By: Claude Opus 5 (1M context) --- src/backend/access/gin/ginget.c | 9 + src/test/recovery/Makefile | 3 +- src/test/recovery/meson.build | 1 + .../recovery/t/056_gin_page_reuse_conflict.pl | 185 ++++++++++++++++++ 4 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 src/test/recovery/t/056_gin_page_reuse_conflict.pl diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c index 2bcb32ca3..89750bdf6 100644 --- a/src/backend/access/gin/ginget.c +++ b/src/backend/access/gin/ginget.c @@ -20,6 +20,7 @@ #include "miscadmin.h" #include "storage/predicate.h" #include "utils/datum.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #include "utils/rel.h" @@ -787,6 +788,14 @@ entryLoadMoreItems(GinState *ginstate, GinScanEntry entry, } else LockBuffer(entry->buffer, GIN_UNLOCK); + + /* + * We return holding only a pin on entry->buffer. The next + * call reads this page's rightlink to step right, so the page + * has to still be the one we left behind. + */ + if (BufferIsValid(entry->buffer)) + INJECTION_POINT("gin-entry-load-more-items-pinned", NULL); return; } } diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile index 9c4102b6b..dfd3617c8 100644 --- a/src/test/recovery/Makefile +++ b/src/test/recovery/Makefile @@ -9,7 +9,8 @@ # #------------------------------------------------------------------------- -EXTRA_INSTALL=contrib/pg_prewarm \ +EXTRA_INSTALL=contrib/pageinspect \ + contrib/pg_prewarm \ 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 39ec8c494..681ac4590 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -64,6 +64,7 @@ tests += { 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', 't/055_cascade_reconnect.pl', + 't/056_gin_page_reuse_conflict.pl', ], }, } diff --git a/src/test/recovery/t/056_gin_page_reuse_conflict.pl b/src/test/recovery/t/056_gin_page_reuse_conflict.pl new file mode 100644 index 000000000..64344ec30 --- /dev/null +++ b/src/test/recovery/t/056_gin_page_reuse_conflict.pl @@ -0,0 +1,185 @@ +# Copyright (c) 2021-2026, PostgreSQL Global Development Group + +# Recycling a deleted GIN index page must raise a recovery conflict on a hot +# standby, as recycling a deleted nbtree or GiST page does. +# +# GIN decides a deleted page is reusable from the primary's horizon alone, and +# writes no WAL when it hands the page out. A standby snapshot is not in that +# horizon, and nothing tells the standby to cancel, so a standby scan holding a +# reference to the page goes on to read whatever the primary put there instead. +# +# Park a standby scan on a posting tree leaf it is holding, hand that page to +# another key on the primary, and ask the resumed scan and a sequential scan the +# same question under the same snapshot. They disagree. +# +# VACUUM prunes the deleted heap tuples in the same pass that empties the index +# pages, and that pruning would cancel the standby query on its own. So the +# snapshot is opened after the DELETE commits, past the pruning horizon. + +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 $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(allows_streaming => 1); +$node_primary->append_conf( + 'postgresql.conf', qq[ +autovacuum = off +# Cancel a conflicting standby query at once, so the test does not have to wait. +max_standby_streaming_delay = 0 +]); +$node_primary->start; + +# The modules may not be installed when this is run under installcheck. +if (!$node_primary->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} +if (!$node_primary->check_extension('pageinspect')) +{ + plan skip_all => 'Extension pageinspect not installed'; +} + +my $backup_name = 'my_backup'; +$node_primary->backup($backup_name); + +my $node_standby = PostgreSQL::Test::Cluster->new('standby'); +$node_standby->init_from_backup($node_primary, $backup_name, + has_streaming => 1); +$node_standby->append_conf( + 'postgresql.conf', qq[ +# The whole point is that the primary must not learn about standby snapshots. +hot_standby_feedback = off +]); +$node_standby->start; + +my $test_db = "test_gin_reuse"; +$node_primary->safe_psql('postgres', "CREATE DATABASE $test_db"); +$node_primary->safe_psql($test_db, "CREATE EXTENSION pageinspect"); +$node_primary->wait_for_replay_catchup($node_standby); + +my $point = 'gin-entry-load-more-items-pinned'; + +$node_primary->safe_psql( + $test_db, qq[ +CREATE EXTENSION injection_points; +CREATE TABLE w (id int, a int[]) WITH (autovacuum_enabled = off); +INSERT INTO w SELECT g, '{1}'::int[] FROM generate_series(1, 120000) g; +CREATE INDEX w_idx ON w USING gin (a) WITH (fastupdate = off); +]); + +# The scan pauses on the second leaf, so that is the page to delete and reuse. +# Empty it before the standby takes its snapshot, so pruning is not what +# cancels the standby query later. +$node_primary->safe_psql( + $test_db, qq[ +CREATE TABLE w_victim AS +SELECT blk FROM ( + SELECT b AS blk, + (SELECT min(t) + FROM gin_leafpage_items(get_raw_page('w_idx', b)) i, + LATERAL unnest(i.tids) t) AS lo + FROM generate_series(0, (pg_relation_size('w_idx') / + current_setting('block_size')::int)::int - 1) b + WHERE (gin_page_opaque_info(get_raw_page('w_idx', b))).flags + = '{data,leaf,compressed}' +) s +ORDER BY lo OFFSET 1 LIMIT 1; + +DELETE FROM w WHERE ctid IN ( + SELECT t + FROM w_victim v, + gin_leafpage_items(get_raw_page('w_idx', v.blk)) i, + LATERAL unnest(i.tids) t); +]); + +# Put the standby's snapshot clear of the pruning horizon. +$node_primary->safe_psql($test_db, "SELECT pg_current_xact_id()") for (1 .. 3); +$node_primary->wait_for_replay_catchup($node_standby); + +$node_standby->safe_psql($test_db, + "SELECT injection_points_attach('$point', 'wait')"); + +my $psql_w = $node_standby->background_psql($test_db, on_error_stop => 0); + +# Take the snapshot with a sequential scan, so it does not touch the GIN index. +$psql_w->query_safe( + "BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM w;"); + +# Fire the index scan and return while it is still running. +$psql_w->query_until( + qr/scanning/, qq[ +\\echo scanning +SET enable_seqscan = off; +SELECT count(*) FROM w WHERE a \@> '{1}'; +]); + +$node_standby->poll_query_until($test_db, + "SELECT count(*) > 0 FROM pg_stat_activity WHERE wait_event = '$point'") + or die "timed out waiting for the standby scan to pause"; + +# Delete the pinned page and put it in the FSM. +$node_primary->safe_psql( + $test_db, qq[ +VACUUM (INDEX_CLEANUP ON) w; +SELECT pg_current_xact_id(); +VACUUM (INDEX_CLEANUP ON) w; +]); +my $deleted = $node_primary->safe_psql($test_db, + "SELECT (gin_page_opaque_info(get_raw_page('w_idx', v.blk))).flags \@> '{deleted}' FROM w_victim v" +); +is($deleted, 't', "the page the standby is pinning was deleted"); + +# Hand it to a different key, which takes it back out of the FSM silently. +$node_primary->safe_psql($test_db, + "INSERT INTO w SELECT 3000000 + g, '{2}'::int[] FROM generate_series(1, 120000) g" +); +my $reused = $node_primary->safe_psql($test_db, + "SELECT NOT ((gin_page_opaque_info(get_raw_page('w_idx', v.blk))).flags \@> '{deleted}') FROM w_victim v" +); +is($reused, 't', "the page was handed to another key while pinned"); + +$node_primary->wait_for_replay_catchup($node_standby); + +# Detach before waking, or the scan just stops at the next call. +$node_standby->safe_psql( + $test_db, qq[ +SELECT injection_points_detach('$point'); +SELECT injection_points_wakeup('$point'); +]); + +# Collect the paused scan's answer, then ask again without the index under the +# same snapshot. The two must agree. +my $out = $psql_w->query( + qq[ +SET enable_seqscan = on; +SET enable_indexscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM w WHERE a \@> '{1}'; +]); +my @counts = ($out =~ /^(\d+)$/mg); + +is(scalar @counts, 2, "collected the index answer and the true answer") + or diag("stdout: $out\nstderr: $psql_w->{stderr}"); + +SKIP: +{ + skip "scan did not return a countable answer", 1 if scalar @counts != 2; + is($counts[0], $counts[1], + "standby index scan agrees with a sequential scan under the same snapshot" + ); +} + +eval { $psql_w->quit }; + +$node_standby->stop(); +$node_primary->stop(); + +done_testing(); -- 2.53.0