From 3c663bfd3fcb62b5a4d4726919be67becc7c1ba5 Mon Sep 17 00:00:00 2001 From: nkey Date: Sat, 23 Nov 2024 13:25:11 +0100 Subject: [PATCH v3 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 points are added in the index_getnext_slot and check_exclusion_or_unique_constraint functions. These injection points allow the test to control the timing of operations, ensuring the race condition is triggered consistently. Changes include: * Added injection points in src/backend/access/index/indexam.c and src/backend/executor/execIndexing.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/backend/executor/execIndexing.c | 1 + src/test/modules/injection_points/Makefile | 2 +- .../expected/dirty_index_scan.out | 39 +++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/dirty_index_scan.spec | 43 +++++++++++++++++++ 6 files changed, 93 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 1859be614c0..1a70de2f470 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/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c index 34e09dee17f..2cf5bd236d4 100644 --- a/src/backend/executor/execIndexing.c +++ b/src/backend/executor/execIndexing.c @@ -810,6 +810,7 @@ check_exclusion_or_unique_constraint(Relation heap, Relation index, * May have to restart scan from this point if a potential conflict is * found. */ + INJECTION_POINT("check_exclusion_or_unique_constraint_before_index_scan"); retry: conflict = false; found_self = false; 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..0451c7513b6 --- /dev/null +++ b/src/test/modules/injection_points/expected/dirty_index_scan.out @@ -0,0 +1,39 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_s1 s3_s1 s2_s1 s3_s2 +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 s3_s1: + SELECT injection_points_detach('check_exclusion_or_unique_constraint_before_index_scan'); + SELECT injection_points_wakeup('check_exclusion_or_unique_constraint_before_index_scan'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_s1: UPDATE test.tbl SET n = n + 1 WHERE i = 42; +step s3_s2: + SELECT injection_points_detach('index_getnext_slot_before_fetch'); + SELECT injection_points_wakeup('index_getnext_slot_before_fetch'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s1_s1: <... completed> diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 58f19001157..26a98bfa148 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..6fb5b985431 --- /dev/null +++ b/src/test/modules/injection_points/specs/dirty_index_scan.spec @@ -0,0 +1,43 @@ +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('check_exclusion_or_unique_constraint_before_index_scan', 'wait'); + 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('check_exclusion_or_unique_constraint_before_index_scan'); + SELECT injection_points_wakeup('check_exclusion_or_unique_constraint_before_index_scan'); +} +step s3_s2 { + SELECT injection_points_detach('index_getnext_slot_before_fetch'); + SELECT injection_points_wakeup('index_getnext_slot_before_fetch'); +} + +permutation + s1_s1 + s3_s1 + s2_s1 + s3_s2 \ No newline at end of file -- 2.43.0