From dca29593e04dceae31c9234a58ef9afa1d7a1ebd Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu Date: Mon, 3 Aug 2026 13:19:55 +0200 Subject: [PATCH v1] hash: create the vacuum read stream before caching the metapage hashbulkdelete() took the metapage from _hash_getcachedmetap(), which returns the relcache's own copy in rel->rd_amcache, and only then created the read stream. read_stream_begin_relation() determines max_ios from the relation's tablespace, and get_tablespace_maintenance_io_concurrency() reads pg_tablespace whenever TableSpaceCacheHash has no entry for it. Opening that catalog accepts invalidation messages, and an invalidation for this index pfrees rd_amcache. Both the local pointer and the copy handed to the read stream callback were then dangling, and BUCKET_TO_BLKNO() computed a block number out of freed memory: ERROR: could not open file "base/5/16513.1" (target block 3722304991): previous segment is only 66 blocks CONTEXT: while vacuuming index "pv2" of relation "public.pv" parallel worker Create the stream first, so the tablespace lookup happens before anything is taken from the relcache. read_stream_reset() does not repeat that lookup, so the refreshes further down the function are not exposed to it. Introduced by bfa3c4f106b; master and REL_19 only. --- src/backend/access/hash/hash.c | 29 ++++---- .../expected/hash-vacuum-metapage.out | 14 ++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/hash-vacuum-metapage.spec | 70 +++++++++++++++++++ 4 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 src/test/isolation/expected/hash-vacuum-metapage.out create mode 100644 src/test/isolation/specs/hash-vacuum-metapage.spec diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 8d8cd30dc38..635e0d2be8c 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -515,6 +515,22 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, tuples_removed = 0; num_index_tuples = 0; + /* + * Set up the streaming read before fetching the cached metapage, not + * after. + * + * It is safe to use batchmode as hash_bulkdelete_read_stream_cb takes no + * locks. + */ + stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE | + READ_STREAM_USE_BATCHING, + info->strategy, + rel, + MAIN_FORKNUM, + hash_bulkdelete_read_stream_cb, + &stream_private, + 0); + /* * We need a copy of the metapage so that we can use its hashm_spares[] * values to compute bucket page addresses, but a cached copy should be @@ -536,19 +552,6 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, stream_private.next_bucket = cur_bucket; stream_private.max_bucket = cur_maxbucket; - /* - * It is safe to use batchmode as hash_bulkdelete_read_stream_cb takes no - * locks. - */ - stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE | - READ_STREAM_USE_BATCHING, - info->strategy, - rel, - MAIN_FORKNUM, - hash_bulkdelete_read_stream_cb, - &stream_private, - 0); - bucket_loop: while (cur_bucket <= cur_maxbucket) { diff --git a/src/test/isolation/expected/hash-vacuum-metapage.out b/src/test/isolation/expected/hash-vacuum-metapage.out new file mode 100644 index 00000000000..d00ee63ac75 --- /dev/null +++ b/src/test/isolation/expected/hash-vacuum-metapage.out @@ -0,0 +1,14 @@ +Parsed test spec with 2 sessions + +starting permutation: warm lock vac inval commit +step warm: SELECT count(*) FROM hashvac; +count +----- + 900 +(1 row) + +step lock: BEGIN; LOCK TABLE pg_tablespace IN ACCESS EXCLUSIVE MODE; +step vac: VACUUM hashvac; +step inval: ALTER INDEX hashvac_idx SET (fillfactor = 90); +step commit: COMMIT; +step vac: <... completed> diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 26abed9f9f0..56ccaecc35d 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -129,3 +129,4 @@ test: lock-nowait test: for-portion-of test: ddl-dependency-locking test: pub-concurrent-drop +test: hash-vacuum-metapage diff --git a/src/test/isolation/specs/hash-vacuum-metapage.spec b/src/test/isolation/specs/hash-vacuum-metapage.spec new file mode 100644 index 00000000000..72ca6a485ad --- /dev/null +++ b/src/test/isolation/specs/hash-vacuum-metapage.spec @@ -0,0 +1,70 @@ +# hashbulkdelete() must not take the relcache's cached metapage before it has +# built its read stream. +# +# - "warm" makes a seq scan, whose own read stream caches the entry for the +# table's tablespace. Without it the vacuum reads pg_tablespace much +# earlier, from lazy_scan_heap()'s read stream, and blocks in the wrong +# place. +# +# - The index is in a tablespace of its own, so the lookup in hashbulkdelete() +# still misses and has to open pg_tablespace. +# +# - s2 holds pg_tablespace locked, which stops the vacuum in LockRelationOid() +# immediately before its AcceptInvalidationMessages() call. s2 then queues +# an invalidation for the index and commits, so the vacuum consumes it at +# exactly the point where the metapage pointer is already held. +# +# ALTER INDEX ... SET (fillfactor) takes ShareUpdateExclusiveLock on the +# index, which does not conflict with the RowExclusiveLock VACUUM holds on it. +# +# Rows are deleted rather than updated, so that dead index entries really +# exist and ambulkdelete() is called at all. + +# Each setup block is one simple query: CREATE TABLESPACE cannot run in a +# transaction block, and a multi-statement block is one. The tablespace +# outlives the test for the same reason -- teardown may appear only once, and +# the table has to go first -- so it is dropped again on the way in. +setup +{ + DROP TABLESPACE IF EXISTS regress_hash_vac_ts; +} + +setup +{ + SET allow_in_place_tablespaces = on; +} + +setup +{ + CREATE TABLESPACE regress_hash_vac_ts LOCATION ''; +} + +setup +{ + CREATE TABLE hashvac (a int); + INSERT INTO hashvac SELECT g FROM generate_series(1, 1000) g; + CREATE INDEX hashvac_idx ON hashvac USING hash (a) + TABLESPACE regress_hash_vac_ts; + DELETE FROM hashvac WHERE a <= 100; +} + +teardown +{ + DROP TABLE hashvac; +} + +session s1 +step warm { SELECT count(*) FROM hashvac; } +step vac { VACUUM hashvac; } + +session s2 +step lock { BEGIN; LOCK TABLE pg_tablespace IN ACCESS EXCLUSIVE MODE; } +step inval { ALTER INDEX hashvac_idx SET (fillfactor = 90); } +step commit { COMMIT; } + +permutation + warm + lock + vac + inval + commit -- 2.43.0