From 2e20bc45afc2a4a530d786d7911ac1aadf57c47a Mon Sep 17 00:00:00 2001 From: nkey Date: Sat, 23 Nov 2024 13:25:11 +0100 Subject: [PATCH v4 2/2] Add isolation test to reproduce dirty snapshot scan issue This commit introduces an isolation test to reliably reproduce the issue where non-MVCC index scans using SnapshotDirty can miss tuples due to concurrent modifications. This situation can lead to incorrect results. To facilitate this test, new injection point added in the index_getnext_slot. Changes include: * Added injection point in src/backend/access/index/indexam.c * Updated Makefile and meson.build to include the new dirty_index_scan isolation test. * Created a new isolation spec dirty_index_scan.spec and its expected output to define and verify the test steps. * This test complements the previous fix by demonstrating the issue and verifying that the fix effectively addresses it. --- src/backend/access/index/indexam.c | 8 ++++ src/test/modules/injection_points/Makefile | 2 +- .../expected/dirty_index_scan.out | 26 +++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/dirty_index_scan.spec | 37 +++++++++++++++++++ 5 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/dirty_index_scan.out create mode 100644 src/test/modules/injection_points/specs/dirty_index_scan.spec diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 8b1f555435b..ad3a3605282 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -57,6 +57,7 @@ #include "utils/ruleutils.h" #include "utils/snapmgr.h" #include "utils/syscache.h" +#include "utils/injection_point.h" /* ---------------------------------------------------------------- @@ -696,6 +697,13 @@ index_getnext_slot(IndexScanDesc scan, ScanDirection direction, TupleTableSlot * * the index. */ Assert(ItemPointerIsValid(&scan->xs_heaptid)); +#ifdef USE_INJECTION_POINTS + if (!IsCatalogRelationOid(scan->indexRelation->rd_id)) + { + INJECTION_POINT("index_getnext_slot_before_fetch"); + } +#endif + if (index_fetch_heap(scan, slot)) return true; } diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 0753a9df58c..11a9bacc750 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -13,7 +13,7 @@ PGFILEDESC = "injection_points - facility for injection points" REGRESS = injection_points reindex_conc REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress -ISOLATION = basic inplace +ISOLATION = basic inplace dirty_index_scan TAP_TESTS = 1 diff --git a/src/test/modules/injection_points/expected/dirty_index_scan.out b/src/test/modules/injection_points/expected/dirty_index_scan.out new file mode 100644 index 00000000000..51ff2d0b0d0 --- /dev/null +++ b/src/test/modules/injection_points/expected/dirty_index_scan.out @@ -0,0 +1,26 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_s1 s2_s1 s3_s1 +injection_points_attach +----------------------- + +(1 row) + +step s1_s1: INSERT INTO test.tbl VALUES(42, 1) on conflict(i) do update set n = EXCLUDED.n + 1; +step s2_s1: UPDATE test.tbl SET n = n + 1 WHERE i = 42; +step s3_s1: + SELECT injection_points_detach('index_getnext_slot_before_fetch'); + SELECT injection_points_wakeup('index_getnext_slot_before_fetch'); + +step s1_s1: <... completed> +step s3_s1: <... completed> +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 989b4db226b..3911aa0274d 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -44,6 +44,7 @@ tests += { 'specs': [ 'basic', 'inplace', + 'dirty_index_scan', ], }, 'tap': { diff --git a/src/test/modules/injection_points/specs/dirty_index_scan.spec b/src/test/modules/injection_points/specs/dirty_index_scan.spec new file mode 100644 index 00000000000..54065f233e4 --- /dev/null +++ b/src/test/modules/injection_points/specs/dirty_index_scan.spec @@ -0,0 +1,37 @@ +setup +{ + CREATE EXTENSION injection_points; + CREATE SCHEMA test; + CREATE UNLOGGED TABLE test.tbl(i int primary key, n int); + CREATE INDEX tbl_n_idx ON test.tbl(n); + INSERT INTO test.tbl VALUES(42,1); +} + +teardown +{ + DROP SCHEMA test CASCADE; + DROP EXTENSION injection_points; +} + +session s1 +setup { + SELECT injection_points_set_local(); + SELECT injection_points_attach('check_exclusion_or_unique_constraint_no_conflict', 'error'); + SELECT injection_points_attach('index_getnext_slot_before_fetch', 'wait'); +} + +step s1_s1 { INSERT INTO test.tbl VALUES(42, 1) on conflict(i) do update set n = EXCLUDED.n + 1; } + +session s2 +step s2_s1 { UPDATE test.tbl SET n = n + 1 WHERE i = 42; } + +session s3 +step s3_s1 { + SELECT injection_points_detach('index_getnext_slot_before_fetch'); + SELECT injection_points_wakeup('index_getnext_slot_before_fetch'); +} + +permutation + s1_s1 + s2_s1 + s3_s1(s1_s1) \ No newline at end of file -- 2.43.0