From 7ed82faee6f1d4bd69d7df56cf6765fffaf3a071 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Fri, 21 Mar 2025 16:41:31 +0100 Subject: [PATCH v14 8/8] Test for IOS/Vacuum race conditions in index AMs Add regression tests that demonstrate wrong results can occur with index-only scans in GiST and SP-GiST indexes when encountering tuples being removed by a concurrent VACUUM operation. With these tests the index AMs are also expected to not block VACUUM even when they're used inside a cursor. Co-authored-by: Matthias van de Meent Co-authored-by: Peter Geoghegan Co-authored-by: Michail Nikolaev Discussion: https://www.postgresql.org/message-id/flat/CANtu0oi0rkR%2BFsgyLXnGZ-uW2950-urApAWLhy-%2BV1WJD%3D_ZXA%40mail.gmail.com --- .../expected/index-only-scan-btree-vacuum.out | 59 +++++++++ .../expected/index-only-scan-gist-vacuum.out | 53 ++++++++ .../index-only-scan-spgist-vacuum.out | 53 ++++++++ src/test/isolation/isolation_schedule | 3 + .../specs/index-only-scan-btree-vacuum.spec | 113 ++++++++++++++++++ .../specs/index-only-scan-gist-vacuum.spec | 112 +++++++++++++++++ .../specs/index-only-scan-spgist-vacuum.spec | 112 +++++++++++++++++ 7 files changed, 505 insertions(+) create mode 100644 src/test/isolation/expected/index-only-scan-btree-vacuum.out create mode 100644 src/test/isolation/expected/index-only-scan-gist-vacuum.out create mode 100644 src/test/isolation/expected/index-only-scan-spgist-vacuum.out create mode 100644 src/test/isolation/specs/index-only-scan-btree-vacuum.spec create mode 100644 src/test/isolation/specs/index-only-scan-gist-vacuum.spec create mode 100644 src/test/isolation/specs/index-only-scan-spgist-vacuum.spec diff --git a/src/test/isolation/expected/index-only-scan-btree-vacuum.out b/src/test/isolation/expected/index-only-scan-btree-vacuum.out new file mode 100644 index 00000000000..9a9d94c86f6 --- /dev/null +++ b/src/test/isolation/expected/index-only-scan-btree-vacuum.out @@ -0,0 +1,59 @@ +Parsed test spec with 2 sessions + +starting permutation: s2_mod s1_begin s1_prepare_sorted_asc s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a BETWEEN 2 AND 9; + +step s1_begin: BEGIN; +step s1_prepare_sorted_asc: + DECLARE foo NO SCROLL CURSOR FOR SELECT a as x FROM ios_needs_cleanup_lock ORDER BY a ASC; + +step s1_fetch_1: + FETCH FROM foo; + +x +- +1 +(1 row) + +step s2_vacuum: + VACUUM (TRUNCATE false) ios_needs_cleanup_lock; + +step s1_fetch_all: + FETCH ALL FROM foo; + + x +-- +10 +(1 row) + +step s1_commit: COMMIT; + +starting permutation: s2_mod s1_begin s1_prepare_sorted_desc s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a BETWEEN 2 AND 9; + +step s1_begin: BEGIN; +step s1_prepare_sorted_desc: + DECLARE foo NO SCROLL CURSOR FOR SELECT a as x FROM ios_needs_cleanup_lock ORDER BY a DESC; + +step s1_fetch_1: + FETCH FROM foo; + + x +-- +10 +(1 row) + +step s2_vacuum: + VACUUM (TRUNCATE false) ios_needs_cleanup_lock; + +step s1_fetch_all: + FETCH ALL FROM foo; + +x +- +1 +(1 row) + +step s1_commit: COMMIT; diff --git a/src/test/isolation/expected/index-only-scan-gist-vacuum.out b/src/test/isolation/expected/index-only-scan-gist-vacuum.out new file mode 100644 index 00000000000..b7c02ee9529 --- /dev/null +++ b/src/test/isolation/expected/index-only-scan-gist-vacuum.out @@ -0,0 +1,53 @@ +Parsed test spec with 2 sessions + +starting permutation: s2_mod s1_begin s1_prepare_sorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; + +step s1_begin: BEGIN; +step s1_prepare_sorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a <-> point '(0,0)' as x FROM ios_needs_cleanup_lock ORDER BY a <-> point '(0,0)'; + +step s1_fetch_1: + FETCH FROM foo; + + x +------------------ +1.4142135623730951 +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + FETCH ALL FROM foo; + +x +- +(0 rows) + +step s1_commit: COMMIT; + +starting permutation: s2_mod s1_begin s1_prepare_unsorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; + +step s1_begin: BEGIN; +step s1_prepare_unsorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE box '((-100,-100),(100,100))' @> a; + +step s1_fetch_1: + FETCH FROM foo; + +a +----- +(1,1) +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + FETCH ALL FROM foo; + +a +- +(0 rows) + +step s1_commit: COMMIT; diff --git a/src/test/isolation/expected/index-only-scan-spgist-vacuum.out b/src/test/isolation/expected/index-only-scan-spgist-vacuum.out new file mode 100644 index 00000000000..b7c02ee9529 --- /dev/null +++ b/src/test/isolation/expected/index-only-scan-spgist-vacuum.out @@ -0,0 +1,53 @@ +Parsed test spec with 2 sessions + +starting permutation: s2_mod s1_begin s1_prepare_sorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; + +step s1_begin: BEGIN; +step s1_prepare_sorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a <-> point '(0,0)' as x FROM ios_needs_cleanup_lock ORDER BY a <-> point '(0,0)'; + +step s1_fetch_1: + FETCH FROM foo; + + x +------------------ +1.4142135623730951 +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + FETCH ALL FROM foo; + +x +- +(0 rows) + +step s1_commit: COMMIT; + +starting permutation: s2_mod s1_begin s1_prepare_unsorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; + +step s1_begin: BEGIN; +step s1_prepare_unsorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE box '((-100,-100),(100,100))' @> a; + +step s1_fetch_1: + FETCH FROM foo; + +a +----- +(1,1) +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + FETCH ALL FROM foo; + +a +- +(0 rows) + +step s1_commit: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index f2e067b1fbc..6366ad23c0d 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -18,6 +18,9 @@ test: two-ids test: multiple-row-versions test: index-only-scan test: index-only-bitmapscan +test: index-only-scan-btree-vacuum +test: index-only-scan-gist-vacuum +test: index-only-scan-spgist-vacuum test: predicate-lock-hot-tuple test: update-conflict-out test: deadlock-simple diff --git a/src/test/isolation/specs/index-only-scan-btree-vacuum.spec b/src/test/isolation/specs/index-only-scan-btree-vacuum.spec new file mode 100644 index 00000000000..9a00804c2c5 --- /dev/null +++ b/src/test/isolation/specs/index-only-scan-btree-vacuum.spec @@ -0,0 +1,113 @@ +# index-only-scan test showing correct results with btree even with concurrent +# vacuum + +setup +{ + -- by using a low fillfactor and a wide tuple we can get multiple blocks + -- with just few rows + CREATE TABLE ios_needs_cleanup_lock (a int NOT NULL, pad char(1024) default '') + WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10); + + INSERT INTO ios_needs_cleanup_lock SELECT g.i FROM generate_series(1, 10) g(i); + + CREATE INDEX ios_btree_a ON ios_needs_cleanup_lock USING btree (a); +} +setup +{ + VACUUM (ANALYZE) ios_needs_cleanup_lock; +} + +teardown +{ + DROP TABLE ios_needs_cleanup_lock; +} + + +session s1 + +# Force an index-only scan, where possible: +setup { + SET enable_bitmapscan = false; + SET enable_indexonlyscan = true; + SET enable_indexscan = true; +} + +step s1_begin { BEGIN; } +step s1_commit { COMMIT; } + +step s1_prepare_sorted_asc { + DECLARE foo NO SCROLL CURSOR FOR SELECT a as x FROM ios_needs_cleanup_lock ORDER BY a ASC; +} +step s1_prepare_sorted_desc { + DECLARE foo NO SCROLL CURSOR FOR SELECT a as x FROM ios_needs_cleanup_lock ORDER BY a DESC; +} + +step s1_fetch_1 { + FETCH FROM foo; +} + +step s1_fetch_all { + FETCH ALL FROM foo; +} + + +session s2 + +# Don't delete row 1, nor 10, so we have a row for the cursor to "rest" on. +step s2_mod +{ + DELETE FROM ios_needs_cleanup_lock WHERE a BETWEEN 2 AND 9; +} + +# Disable truncation, as otherwise we'll just wait for a timeout while trying +# to acquire the lock +step s2_vacuum +{ + VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +} + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_sorted_asc + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # with the bug this vacuum will mark pages as all-visible that the scan in + # the next step then considers all-visible, despite all rows from those + # pages having been removed. + # Because this should block on buffer-level locks, this won't ever be + # considered "blocked" by isolation tester, and so we only have a single + # step we can work with concurrently. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_sorted_desc + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # If the index scan doesn't correctly interlock its visibility tests with + # concurrent VACUUM cleanup then VACUUM will mark pages as all-visible that + # the scan in the next steps may then consider all-visible, despite some of + # those rows having been removed. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit diff --git a/src/test/isolation/specs/index-only-scan-gist-vacuum.spec b/src/test/isolation/specs/index-only-scan-gist-vacuum.spec new file mode 100644 index 00000000000..9d241b25920 --- /dev/null +++ b/src/test/isolation/specs/index-only-scan-gist-vacuum.spec @@ -0,0 +1,112 @@ +# index-only-scan test showing wrong results with GiST +# +setup +{ + -- by using a low fillfactor and a wide tuple we can get multiple blocks + -- with just few rows + CREATE TABLE ios_needs_cleanup_lock (a point NOT NULL, b int not null, pad char(1024) default '') + WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10); + + INSERT INTO ios_needs_cleanup_lock SELECT point(g.i, g.i), g.i FROM generate_series(1, 10) g(i); + + CREATE INDEX ios_spgist_a ON ios_needs_cleanup_lock USING gist(a); +} +setup +{ + VACUUM (ANALYZE) ios_needs_cleanup_lock; +} + +teardown +{ + DROP TABLE ios_needs_cleanup_lock; +} + + +session s1 + +# Force an index-only scan, where possible: +setup { + SET enable_bitmapscan = false; + SET enable_indexonlyscan = true; + SET enable_indexscan = true; +} + +step s1_begin { BEGIN; } +step s1_commit { COMMIT; } + +step s1_prepare_sorted { + DECLARE foo NO SCROLL CURSOR FOR SELECT a <-> point '(0,0)' as x FROM ios_needs_cleanup_lock ORDER BY a <-> point '(0,0)'; +} + +step s1_prepare_unsorted { + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE box '((-100,-100),(100,100))' @> a; +} + +step s1_fetch_1 { + FETCH FROM foo; +} + +step s1_fetch_all { + FETCH ALL FROM foo; +} + + +session s2 + +# Don't delete row 1 so we have a row for the cursor to "rest" on. +step s2_mod +{ + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; +} + +# Disable truncation, as otherwise we'll just wait for a timeout while trying +# to acquire the lock +step s2_vacuum { VACUUM (TRUNCATE false) ios_needs_cleanup_lock; } + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_sorted + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # with the bug this vacuum will mark pages as all-visible that the scan in + # the next step then considers all-visible, despite all rows from those + # pages having been removed. + # Because this should block on buffer-level locks, this won't ever be + # considered "blocked" by isolation tester, and so we only have a single + # step we can work with concurrently. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_unsorted + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # with the bug this vacuum will mark pages as all-visible that the scan in + # the next step then considers all-visible, despite all rows from those + # pages having been removed. + # Because this should block on buffer-level locks, this won't ever be + # considered "blocked" by isolation tester, and so we only have a single + # step we can work with concurrently. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit diff --git a/src/test/isolation/specs/index-only-scan-spgist-vacuum.spec b/src/test/isolation/specs/index-only-scan-spgist-vacuum.spec new file mode 100644 index 00000000000..cd621d4f7f2 --- /dev/null +++ b/src/test/isolation/specs/index-only-scan-spgist-vacuum.spec @@ -0,0 +1,112 @@ +# index-only-scan test showing wrong results with SPGiST +# +setup +{ + -- by using a low fillfactor and a wide tuple we can get multiple blocks + -- with just few rows + CREATE TABLE ios_needs_cleanup_lock (a point NOT NULL, b int not null, pad char(1024) default '') + WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10); + + INSERT INTO ios_needs_cleanup_lock SELECT point(g.i, g.i), g.i FROM generate_series(1, 10) g(i); + + CREATE INDEX ios_spgist_a ON ios_needs_cleanup_lock USING spgist(a); +} +setup +{ + VACUUM (ANALYZE) ios_needs_cleanup_lock; +} + +teardown +{ + DROP TABLE ios_needs_cleanup_lock; +} + + +session s1 + +# Force an index-only scan, where possible: +setup { + SET enable_bitmapscan = false; + SET enable_indexonlyscan = true; + SET enable_indexscan = true; +} + +step s1_begin { BEGIN; } +step s1_commit { COMMIT; } + +step s1_prepare_sorted { + DECLARE foo NO SCROLL CURSOR FOR SELECT a <-> point '(0,0)' as x FROM ios_needs_cleanup_lock ORDER BY a <-> point '(0,0)'; +} + +step s1_prepare_unsorted { + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE box '((-100,-100),(100,100))' @> a; +} + +step s1_fetch_1 { + FETCH FROM foo; +} + +step s1_fetch_all { + FETCH ALL FROM foo; +} + + +session s2 + +# Don't delete row 1 so we have a row for the cursor to "rest" on. +step s2_mod +{ + DELETE FROM ios_needs_cleanup_lock WHERE a != point '(1,1)'; +} + +# Disable truncation, as otherwise we'll just wait for a timeout while trying +# to acquire the lock +step s2_vacuum { VACUUM (TRUNCATE false) ios_needs_cleanup_lock; } + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_sorted + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # with the bug this vacuum will mark pages as all-visible that the scan in + # the next step then considers all-visible, despite all rows from those + # pages having been removed. + # Because this should block on buffer-level locks, this won't ever be + # considered "blocked" by isolation tester, and so we only have a single + # step we can work with concurrently. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit + +permutation + # delete nearly all rows, to make issue visible + s2_mod + # create a cursor + s1_begin + s1_prepare_unsorted + + # fetch one row from the cursor, that ensures the index scan portion is done + # before the vacuum in the next step + s1_fetch_1 + + # with the bug this vacuum will mark pages as all-visible that the scan in + # the next step then considers all-visible, despite all rows from those + # pages having been removed. + # Because this should block on buffer-level locks, this won't ever be + # considered "blocked" by isolation tester, and so we only have a single + # step we can work with concurrently. + s2_vacuum + + # if this returns any rows, we're busted + s1_fetch_all + + s1_commit -- 2.50.1 (Apple Git-155)