From afa0310803bf72bdaccff265afbdecf26da88435 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Mon, 2 Dec 2024 15:50:04 -0500 Subject: [PATCH v3 1/2] isolationtester showing broken index-only scans with GiST v3 Co-authored-by: Matthias van de Meent --- contrib/btree_gist/.gitignore | 2 + contrib/btree_gist/Makefile | 3 + contrib/btree_gist/expected/btree_gist.out | 69 ++++++++++++ contrib/btree_gist/meson.build | 6 ++ contrib/btree_gist/specs/btree_gist.spec | 117 +++++++++++++++++++++ 5 files changed, 197 insertions(+) create mode 100644 contrib/btree_gist/expected/btree_gist.out create mode 100644 contrib/btree_gist/specs/btree_gist.spec diff --git a/contrib/btree_gist/.gitignore b/contrib/btree_gist/.gitignore index 5dcb3ff9723..b4903eba657 100644 --- a/contrib/btree_gist/.gitignore +++ b/contrib/btree_gist/.gitignore @@ -1,4 +1,6 @@ # Generated subdirectories /log/ /results/ +/output_iso/ /tmp_check/ +/tmp_check_iso/ diff --git a/contrib/btree_gist/Makefile b/contrib/btree_gist/Makefile index 7ac2df26c10..e8cdef2277d 100644 --- a/contrib/btree_gist/Makefile +++ b/contrib/btree_gist/Makefile @@ -42,6 +42,9 @@ REGRESS = init int2 int4 int8 float4 float8 cash oid timestamp timestamptz \ bytea bit varbit numeric uuid not_equal enum bool partitions \ stratnum without_overlaps +ISOLATION = btree_gist +ISOLATION_OPTS = --load-extension=btree_gist + SHLIB_LINK += $(filter -lm, $(LIBS)) ifdef USE_PGXS diff --git a/contrib/btree_gist/expected/btree_gist.out b/contrib/btree_gist/expected/btree_gist.out new file mode 100644 index 00000000000..88dad12a415 --- /dev/null +++ b/contrib/btree_gist/expected/btree_gist.out @@ -0,0 +1,69 @@ +Parsed test spec with 2 sessions + +starting permutation: s2_vacuum s2_mod s1_begin s1_prepare_sorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a > 1; + +step s1_begin: BEGIN; +step s1_prepare_sorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0 ORDER BY a <-> 0; + +step s1_fetch_1: + FETCH FROM foo; + +a +- +1 +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + SELECT pg_sleep_for(INTERVAL '50ms'); + FETCH ALL FROM foo; + +pg_sleep_for +------------ + +(1 row) + +a +- +(0 rows) + +step s2_vacuum: <... completed> +step s1_commit: COMMIT; + +starting permutation: s2_vacuum s2_mod s1_begin s1_prepare_unsorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s2_mod: + DELETE FROM ios_needs_cleanup_lock WHERE a > 1; + +step s1_begin: BEGIN; +step s1_prepare_unsorted: + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0; + +step s1_fetch_1: + FETCH FROM foo; + +a +- +1 +(1 row) + +step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; +step s1_fetch_all: + SELECT pg_sleep_for(INTERVAL '50ms'); + FETCH ALL FROM foo; + +pg_sleep_for +------------ + +(1 row) + +a +- +(0 rows) + +step s2_vacuum: <... completed> +step s1_commit: COMMIT; diff --git a/contrib/btree_gist/meson.build b/contrib/btree_gist/meson.build index 73b1bbf52a6..51adf635eb9 100644 --- a/contrib/btree_gist/meson.build +++ b/contrib/btree_gist/meson.build @@ -94,4 +94,10 @@ tests += { 'without_overlaps', ], }, + 'isolation': { + 'specs': [ + 'btree_gist', + ], + 'regress_args': ['--load-extension=btree_gist'], + }, } diff --git a/contrib/btree_gist/specs/btree_gist.spec b/contrib/btree_gist/specs/btree_gist.spec new file mode 100644 index 00000000000..18ad7b4dbd5 --- /dev/null +++ b/contrib/btree_gist/specs/btree_gist.spec @@ -0,0 +1,117 @@ +# 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 int NOT NULL, b int not null, pad char(1024) default '') + WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10); + + INSERT INTO ios_needs_cleanup_lock SELECT g.i, g.i FROM generate_series(1, 10) g(i); + + CREATE INDEX ios_gist_a ON ios_needs_cleanup_lock USING gist(a); +} + +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 FROM ios_needs_cleanup_lock WHERE a > 0 ORDER BY a <-> 0; +} + +step s1_prepare_unsorted { + DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0; +} + +step s1_fetch_1 { + FETCH FROM foo; +} + +step s1_fetch_all { + SELECT pg_sleep_for(INTERVAL '50ms'); + 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 > 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 + # Vacuum first, to ensure VM exists, otherwise the bitmapscan will consider + # VM to be size 0, due to caching. Can't do that in setup because + s2_vacuum + + # 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 + # Vacuum first, to ensure VM exists, otherwise the bitmapscan will consider + # VM to be size 0, due to caching. Can't do that in setup because + s2_vacuum + + # 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.45.2