From 3131a69bd8b7fffa450795fae573ee2fb8c47288 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Wed, 8 Jul 2026 15:18:06 -0400 Subject: [PATCH v1 2/2] Add tests for hash index scans concurrent with bucket splits. Test the bug fixed by the preceding commit at two levels, using two new injection points in _hash_splitbucket: an error-mode point placed before tuple relocation begins (where existing code can fail anyway), and a wait-mode point placed after tuple relocation, where the splitter holds no buffer content locks. A regress test uses the error-mode point to interrupt a split partway through, then verifies that forward, backward, and bitmap scans of the affected bucket pair all agree with a seqscan. The bug isn't specific to interrupted splits: interrupting a split is just the most convenient way to hold the bucket pair in the same state that scans observe while any split is in progress, making the test deterministic without a second session. An isolation test proves that claim directly: it pauses a splitter on the wait-mode point and runs the same scans from a second session while the split remains genuinely in progress, then lets the split finish and scans once more. Co-Authored-By: Claude Fable 5 --- src/backend/access/hash/hashpage.c | 5 + src/test/modules/injection_points/Makefile | 3 +- .../injection_points/expected/hash-split.out | 70 ++++++++++ .../injection_points/expected/hash_split.out | 131 ++++++++++++++++++ src/test/modules/injection_points/meson.build | 2 + .../injection_points/specs/hash-split.spec | 81 +++++++++++ .../injection_points/sql/hash_split.sql | 80 +++++++++++ 7 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/hash-split.out create mode 100644 src/test/modules/injection_points/expected/hash_split.out create mode 100644 src/test/modules/injection_points/specs/hash-split.spec create mode 100644 src/test/modules/injection_points/sql/hash_split.sql diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index 8099b0d02..c2e7213c4 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -35,6 +35,7 @@ #include "port/pg_bitutils.h" #include "storage/predicate.h" #include "storage/smgr.h" +#include "utils/injection_point.h" #include "utils/rel.h" static bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock, @@ -1104,6 +1105,8 @@ _hash_splitbucket(Relation rel, npage = BufferGetPage(nbuf); nopaque = HashPageGetOpaque(npage); + INJECTION_POINT("hash-split-before-relocation", NULL); + /* Copy the predicate locks from old bucket to new bucket. */ PredicateLockPageSplit(rel, BufferGetBlockNumber(bucket_obuf), @@ -1252,6 +1255,8 @@ _hash_splitbucket(Relation rel, /* be tidy */ for (i = 0; i < nitups; i++) pfree(itups[i]); + + INJECTION_POINT("hash-split-after-relocation", NULL); break; } diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index c01d2fb09..23e00207b 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -9,10 +9,11 @@ EXTENSION = injection_points DATA = injection_points--1.0.sql PGFILEDESC = "injection_points - facility for injection points" -REGRESS = injection_points hashagg reindex_conc vacuum +REGRESS = injection_points hashagg reindex_conc vacuum hash_split REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ + hash-split \ inplace \ repack \ repack_temporal \ diff --git a/src/test/modules/injection_points/expected/hash-split.out b/src/test/modules/injection_points/expected/hash-split.out new file mode 100644 index 000000000..d087a2c69 --- /dev/null +++ b/src/test/modules/injection_points/expected/hash-split.out @@ -0,0 +1,70 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_split s2_insert s2_scan s2_back s2_detach s2_wakeup s2_scan +injection_points_attach +----------------------- + +(1 row) + +step s1_split: + INSERT INTO hash_split_test + SELECT 1000000 + g FROM hash_split_geom, generate_series(1, 320 * nbuckets) g; + +step s2_insert: + INSERT INTO hash_split_test SELECT k FROM hash_split_key; + +step s2_scan: + SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + +count +----- + 11 +(1 row) + +step s2_back: + BEGIN; + DECLARE c SCROLL CURSOR FOR + SELECT v = (SELECT k FROM hash_split_key) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + MOVE FORWARD ALL IN c; + FETCH BACKWARD ALL FROM c; + COMMIT; + +?column? +-------- +t +t +t +t +t +t +t +t +t +t +t +(11 rows) + +step s2_detach: SELECT injection_points_detach('hash-split-after-relocation'); +injection_points_detach +----------------------- + +(1 row) + +step s2_wakeup: SELECT injection_points_wakeup('hash-split-after-relocation'); +injection_points_wakeup +----------------------- + +(1 row) + +step s1_split: <... completed> +step s2_scan: + SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + +count +----- + 11 +(1 row) + diff --git a/src/test/modules/injection_points/expected/hash_split.out b/src/test/modules/injection_points/expected/hash_split.out new file mode 100644 index 000000000..0754618c2 --- /dev/null +++ b/src/test/modules/injection_points/expected/hash_split.out @@ -0,0 +1,131 @@ +-- Test hash index scans of a bucket pair whose split is incomplete +-- +-- The behavior tested here is not specific to incomplete splits: it +-- applies to any scan that runs while the bucket pair's split-in-progress +-- flags are set, which is the case for the duration of every split. +-- Interrupting a split is merely the most convenient way to hold the +-- bucket pair in that state: it makes the tests below deterministic, +-- without any need for a second session. The hash-split isolation test +-- provides equivalent coverage for scans that run during a live split. +CREATE EXTENSION injection_points; +SELECT injection_points_set_local(); + injection_points_set_local +---------------------------- + +(1 row) + +CREATE TABLE hash_split_test (v int4) WITH (autovacuum_enabled = false); +CREATE INDEX hash_split_index ON hash_split_test USING hash (v); +-- Determine the layout of the (never-split) index: one metapage, nbuckets +-- bucket pages, and one bitmap page. The index's first bucket split will +-- split bucket 0, creating bucket nbuckets. +CREATE TEMP TABLE hash_split_geom AS + SELECT pg_relation_size('hash_split_index') / + current_setting('block_size')::bigint - 2 AS nbuckets; +-- sanity: initial bucket counts are always powers of two +SELECT nbuckets > 0 AND (nbuckets & (nbuckets - 1)) = 0 AS nbuckets_ok + FROM hash_split_geom; + nbuckets_ok +------------- + t +(1 row) + +-- Choose a value that maps to bucket 0 before the split and to the new +-- bucket afterwards +CREATE TEMP TABLE hash_split_key AS + SELECT min(v)::int4 AS k + FROM generate_series(1, 10000) v, hash_split_geom + WHERE (hashint4(v::int4) & (2 * nbuckets - 1)) = nbuckets; +INSERT INTO hash_split_test + SELECT k FROM hash_split_key, generate_series(1, 10); +-- Error out during the first bucket split, leaving it incomplete +SELECT injection_points_attach('hash-split-before-relocation', 'error'); + injection_points_attach +------------------------- + +(1 row) + +INSERT INTO hash_split_test + SELECT 1000000 + g FROM hash_split_geom, generate_series(1, 350 * nbuckets) g; +ERROR: error triggered for injection point hash-split-before-relocation +SELECT injection_points_detach('hash-split-before-relocation'); + injection_points_detach +------------------------- + +(1 row) + +-- This insertion goes to the new bucket, which is still flagged as being +-- populated by the incomplete split +INSERT INTO hash_split_test SELECT k FROM hash_split_key; +-- Scans of that bucket must visit both buckets of the incomplete split: +-- the fresh row from the new bucket, plus the 10 older rows that remain +-- in bucket 0 +SET enable_seqscan = off; +SET enable_bitmapscan = off; +EXPLAIN (COSTS OFF) + SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + QUERY PLAN +------------------------------------------------------------ + Aggregate + InitPlan expr_1 + -> Seq Scan on hash_split_key + Disabled: true + -> Index Scan using hash_split_index on hash_split_test + Index Cond: (v = (InitPlan expr_1).col1) +(6 rows) + +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + count +------- + 11 +(1 row) + +-- Backward scans must visit both buckets too, in the opposite order +BEGIN; +DECLARE c SCROLL CURSOR FOR + SELECT v = (SELECT k FROM hash_split_key) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +MOVE FORWARD ALL IN c; +FETCH BACKWARD ALL FROM c; + ?column? +---------- + t + t + t + t + t + t + t + t + t + t + t +(11 rows) + +COMMIT; +-- Same count via a bitmap scan, which shares the underlying scan code +SET enable_indexscan = off; +SET enable_bitmapscan = on; +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + count +------- + 11 +(1 row) + +-- And the ground truth, via the heap +SET enable_bitmapscan = off; +RESET enable_seqscan; +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + count +------- + 11 +(1 row) + +RESET enable_indexscan; +RESET enable_bitmapscan; +DROP TABLE hash_split_test; +DROP EXTENSION injection_points; diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 59dba1cb0..a0b0c4827 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -36,6 +36,7 @@ tests += { 'hashagg', 'reindex_conc', 'vacuum', + 'hash_split', ], 'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/regress'], # The injection points are cluster-wide, so disable installcheck @@ -44,6 +45,7 @@ tests += { 'isolation': { 'specs': [ 'basic', + 'hash-split', 'inplace', 'repack', 'repack_temporal', diff --git a/src/test/modules/injection_points/specs/hash-split.spec b/src/test/modules/injection_points/specs/hash-split.spec new file mode 100644 index 000000000..cfdc0d944 --- /dev/null +++ b/src/test/modules/injection_points/specs/hash-split.spec @@ -0,0 +1,81 @@ +# Test hash index scans that run while a bucket split is in progress. +# +# The interesting window opens once the split has relocated tuples to the +# new bucket, before the buckets' split-in-progress flags are cleared. A +# concurrent scan whose value belongs to the new bucket must visit both +# buckets of the pair: it skips the moved-by-split tuples in the new +# bucket, so it has to read their authoritative copies from the old +# bucket, plus any tuples inserted into the new bucket after the split +# began. +# +# s1 performs the split, pausing inside that window on a wait injection +# point (placed where the splitter holds no buffer content locks). s2 +# then inserts a matching row (which goes to the new bucket) and scans, +# both forwards and backwards. + +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE hash_split_test (v int4) WITH (autovacuum_enabled = false); + CREATE INDEX hash_split_index ON hash_split_test USING hash (v); + CREATE TABLE hash_split_geom AS + SELECT pg_relation_size('hash_split_index') / + current_setting('block_size')::bigint - 2 AS nbuckets; + CREATE TABLE hash_split_key AS + SELECT min(v)::int4 AS k + FROM generate_series(1, 10000) v, hash_split_geom + WHERE (hashint4(v::int4) & (2 * nbuckets - 1)) = nbuckets; + INSERT INTO hash_split_test + SELECT k FROM hash_split_key, generate_series(1, 10); +} + +teardown +{ + DROP TABLE hash_split_test, hash_split_geom, hash_split_key; + DROP EXTENSION injection_points; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('hash-split-after-relocation', 'wait'); +} +step s1_split +{ + INSERT INTO hash_split_test + SELECT 1000000 + g FROM hash_split_geom, generate_series(1, 320 * nbuckets) g; +} + +session s2 +setup +{ + SET enable_seqscan = off; + SET enable_bitmapscan = off; +} +step s2_insert +{ + INSERT INTO hash_split_test SELECT k FROM hash_split_key; +} +step s2_scan +{ + SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +} +step s2_back +{ + BEGIN; + DECLARE c SCROLL CURSOR FOR + SELECT v = (SELECT k FROM hash_split_key) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + MOVE FORWARD ALL IN c; + FETCH BACKWARD ALL FROM c; + COMMIT; +} +step s2_detach { SELECT injection_points_detach('hash-split-after-relocation'); } +step s2_wakeup { SELECT injection_points_wakeup('hash-split-after-relocation'); } + +# Scan during the paused split, then let the split finish and scan again. +# The detach must happen before the wakeup: the same INSERT can trigger +# another split, which must not hit the injection point again. +permutation s1_split s2_insert s2_scan s2_back s2_detach s2_wakeup s2_scan diff --git a/src/test/modules/injection_points/sql/hash_split.sql b/src/test/modules/injection_points/sql/hash_split.sql new file mode 100644 index 000000000..010b65882 --- /dev/null +++ b/src/test/modules/injection_points/sql/hash_split.sql @@ -0,0 +1,80 @@ +-- Test hash index scans of a bucket pair whose split is incomplete +-- +-- The behavior tested here is not specific to incomplete splits: it +-- applies to any scan that runs while the bucket pair's split-in-progress +-- flags are set, which is the case for the duration of every split. +-- Interrupting a split is merely the most convenient way to hold the +-- bucket pair in that state: it makes the tests below deterministic, +-- without any need for a second session. The hash-split isolation test +-- provides equivalent coverage for scans that run during a live split. +CREATE EXTENSION injection_points; +SELECT injection_points_set_local(); + +CREATE TABLE hash_split_test (v int4) WITH (autovacuum_enabled = false); +CREATE INDEX hash_split_index ON hash_split_test USING hash (v); + +-- Determine the layout of the (never-split) index: one metapage, nbuckets +-- bucket pages, and one bitmap page. The index's first bucket split will +-- split bucket 0, creating bucket nbuckets. +CREATE TEMP TABLE hash_split_geom AS + SELECT pg_relation_size('hash_split_index') / + current_setting('block_size')::bigint - 2 AS nbuckets; +-- sanity: initial bucket counts are always powers of two +SELECT nbuckets > 0 AND (nbuckets & (nbuckets - 1)) = 0 AS nbuckets_ok + FROM hash_split_geom; + +-- Choose a value that maps to bucket 0 before the split and to the new +-- bucket afterwards +CREATE TEMP TABLE hash_split_key AS + SELECT min(v)::int4 AS k + FROM generate_series(1, 10000) v, hash_split_geom + WHERE (hashint4(v::int4) & (2 * nbuckets - 1)) = nbuckets; + +INSERT INTO hash_split_test + SELECT k FROM hash_split_key, generate_series(1, 10); + +-- Error out during the first bucket split, leaving it incomplete +SELECT injection_points_attach('hash-split-before-relocation', 'error'); +INSERT INTO hash_split_test + SELECT 1000000 + g FROM hash_split_geom, generate_series(1, 350 * nbuckets) g; +SELECT injection_points_detach('hash-split-before-relocation'); + +-- This insertion goes to the new bucket, which is still flagged as being +-- populated by the incomplete split +INSERT INTO hash_split_test SELECT k FROM hash_split_key; + +-- Scans of that bucket must visit both buckets of the incomplete split: +-- the fresh row from the new bucket, plus the 10 older rows that remain +-- in bucket 0 +SET enable_seqscan = off; +SET enable_bitmapscan = off; +EXPLAIN (COSTS OFF) + SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); + +-- Backward scans must visit both buckets too, in the opposite order +BEGIN; +DECLARE c SCROLL CURSOR FOR + SELECT v = (SELECT k FROM hash_split_key) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +MOVE FORWARD ALL IN c; +FETCH BACKWARD ALL FROM c; +COMMIT; + +-- Same count via a bitmap scan, which shares the underlying scan code +SET enable_indexscan = off; +SET enable_bitmapscan = on; +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +-- And the ground truth, via the heap +SET enable_bitmapscan = off; +RESET enable_seqscan; +SELECT count(*) FROM hash_split_test + WHERE v = (SELECT k FROM hash_split_key); +RESET enable_indexscan; +RESET enable_bitmapscan; + +DROP TABLE hash_split_test; +DROP EXTENSION injection_points; -- 2.53.0