Hello! On Sun, Nov 30, 2025 at 6:26 PM Mihail Nikalayeu <[email protected]> wrote: > I think it is better to implement something in isolationtester to > natively support the case from [0] (in such case we don't need NOTICE > on .pre-invalidate-catalog-snapshot-end).
I think I have implemented a better solution - without any additional NOTICE. It just actually waits for the other backend to hang on the injection point. Attached (together with other changes).
From 5dc3e4eb50e445a291a13663fc9ce93d0db96b1c Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu <[email protected]> Date: Sun, 30 Nov 2025 16:49:20 +0100 Subject: [PATCH v16 2/3] Revert "Doc: cover index CONCURRENTLY causing errors in INSERT ... ON CONFLICT." This reverts commit 8b18ed6dfbb8b3e4483801b513fea6b429140569. --- doc/src/sgml/ref/insert.sgml | 9 --------- src/backend/optimizer/util/plancat.c | 5 ----- 2 files changed, 14 deletions(-) diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index 0598b8dea34..04962e39e12 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -594,15 +594,6 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac </para> </tip> - <warning> - <para> - While <command>CREATE INDEX CONCURRENTLY</command> or <command>REINDEX - CONCURRENTLY</command> is running on a unique index, <command>INSERT - ... ON CONFLICT</command> statements on the same table may unexpectedly - fail with a unique violation. - </para> - </warning> - </refsect2> </refsect1> diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 7af9a2064e3..d5325f0f732 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -789,11 +789,6 @@ find_relation_notnullatts(PlannerInfo *root, Oid relid) * the purposes of inference. If no opclass (or collation) is specified, then * all matching indexes (that may or may not match the default in terms of * each attribute opclass/collation) are used for inference. - * - * Note: during index CONCURRENTLY operations, different transactions may - * reference different sets of arbiter indexes. This can lead to false unique - * constraint violations that wouldn't occur during normal operations. For - * more information, see insert.sgml. */ List * infer_arbiter_indexes(PlannerInfo *root) -- 2.43.0
From a09dcb5f2a0fb5801f205d4f5e0448ddbe064d38 Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu <[email protected]> Date: Mon, 1 Dec 2025 12:14:13 +0100 Subject: [PATCH v16 3/3] Refactor test to avoid any additional injection points and notice. --- src/backend/utils/time/snapmgr.c | 1 - .../index-concurrently-upsert-predicate.out | 8 +- .../index-concurrently-upsert-predicate_1.out | 116 ------------------ .../expected/index-concurrently-upsert.out | 14 +-- .../expected/index-concurrently-upsert_1.out | 116 ------------------ .../index-concurrently-upsert-predicate.spec | 31 ++++- .../specs/index-concurrently-upsert.spec | 36 +++++- 7 files changed, 64 insertions(+), 258 deletions(-) delete mode 100644 src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out delete mode 100644 src/test/modules/injection_points/expected/index-concurrently-upsert_1.out diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 434abbf6b6f..24f73a49d27 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -459,7 +459,6 @@ InvalidateCatalogSnapshot(void) pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node); CatalogSnapshot = NULL; SnapshotResetXmin(); - INJECTION_POINT("pre-invalidate-catalog-snapshot-end", NULL); INJECTION_POINT("invalidate-catalog-snapshot-end", NULL); } } diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out index 5a57cd5fa40..11b23e8721f 100644 --- a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out +++ b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out @@ -1,6 +1,6 @@ Parsed test spec with 5 sessions -starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 +starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 injection_points_attach ----------------------- @@ -37,16 +37,12 @@ case (1 row) -step s5_noop: - <waiting ...> step s3_start_create_index: CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_special_duplicate ON test.tbl(abs(i)) WHERE i < 10000; <waiting ...> -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end step s1_start_upsert: INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now(); <waiting ...> -step s5_noop: <... completed> step s4_wakeup_define_index_before_set_valid: SELECT injection_points_detach('define-index-before-set-valid'); SELECT injection_points_wakeup('define-index-before-set-valid'); @@ -65,6 +61,7 @@ step s2_start_upsert: INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now(); <waiting ...> step s5_wakeup_s1_from_invalidate_catalog_snapshot: + CALL test.wait_for_injection_point(); SELECT injection_points_detach('invalidate-catalog-snapshot-end'); SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); @@ -106,7 +103,6 @@ injection_points_wakeup (1 row) -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end step s1_start_upsert: <... completed> step s2_start_upsert: <... completed> step s3_start_create_index: <... completed> diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out deleted file mode 100644 index d453dd62617..00000000000 --- a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out +++ /dev/null @@ -1,116 +0,0 @@ -Parsed test spec with 5 sessions - -starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -injection_points_attach ------------------------ - -(1 row) - -injection_points_attach ------------------------ - -(1 row) - -injection_points_attach ------------------------ - -(1 row) - -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_attach_invalidate_catalog_snapshot: - SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait'); - <waiting ...> -step s4_wakeup_s1_setup: - SELECT CASE WHEN - (SELECT pid FROM pg_stat_activity - WHERE wait_event_type = 'InjectionPoint' AND - wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL - THEN injection_points_wakeup('invalidate-catalog-snapshot-end') - END; - -case ----- - -(1 row) - -step s1_attach_invalidate_catalog_snapshot: <... completed> -injection_points_attach ------------------------ - -(1 row) - -step s5_noop: - <waiting ...> -step s3_start_create_index: - CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_special_duplicate ON test.tbl(abs(i)) WHERE i < 10000; - <waiting ...> -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_start_upsert: - INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now(); - <waiting ...> -step s5_noop: <... completed> -step s4_wakeup_define_index_before_set_valid: - SELECT injection_points_detach('define-index-before-set-valid'); - SELECT injection_points_wakeup('define-index-before-set-valid'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s2_start_upsert: - INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now(); - <waiting ...> -step s5_wakeup_s1_from_invalidate_catalog_snapshot: - SELECT injection_points_detach('invalidate-catalog-snapshot-end'); - SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s4_wakeup_s2: - SELECT injection_points_detach('exec-insert-before-insert-speculative'); - SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s4_wakeup_s1: - SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); - SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_start_upsert: <... completed> -step s2_start_upsert: <... completed> -step s3_start_create_index: <... completed> diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert.out b/src/test/modules/injection_points/expected/index-concurrently-upsert.out index 97386a35bed..7bcf8d34053 100644 --- a/src/test/modules/injection_points/expected/index-concurrently-upsert.out +++ b/src/test/modules/injection_points/expected/index-concurrently-upsert.out @@ -1,6 +1,6 @@ Parsed test spec with 5 sessions -starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 +starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 injection_points_attach ----------------------- @@ -27,8 +27,8 @@ injection_points_attach step s4_wakeup_s1_setup: SELECT CASE WHEN (SELECT pid FROM pg_stat_activity - WHERE wait_event_type = 'InjectionPoint' AND - wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL + WHERE wait_event_type = 'InjectionPoint' AND + wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL THEN injection_points_wakeup('invalidate-catalog-snapshot-end') END; @@ -37,16 +37,12 @@ case (1 row) -step s5_noop: - <waiting ...> step s3_start_create_index: CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_duplicate ON test.tbl(i); <waiting ...> -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end step s1_start_upsert: - INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); + INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); <waiting ...> -step s5_noop: <... completed> step s4_wakeup_define_index_before_set_valid: SELECT injection_points_detach('define-index-before-set-valid'); SELECT injection_points_wakeup('define-index-before-set-valid'); @@ -65,6 +61,7 @@ step s2_start_upsert: INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); <waiting ...> step s5_wakeup_s1_from_invalidate_catalog_snapshot: + CALL test.wait_for_injection_point(); SELECT injection_points_detach('invalidate-catalog-snapshot-end'); SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); @@ -106,7 +103,6 @@ injection_points_wakeup (1 row) -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end step s1_start_upsert: <... completed> step s2_start_upsert: <... completed> step s3_start_create_index: <... completed> diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out b/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out deleted file mode 100644 index 4bd51b2b511..00000000000 --- a/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out +++ /dev/null @@ -1,116 +0,0 @@ -Parsed test spec with 5 sessions - -starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1 -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -injection_points_attach ------------------------ - -(1 row) - -injection_points_attach ------------------------ - -(1 row) - -injection_points_attach ------------------------ - -(1 row) - -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_attach_invalidate_catalog_snapshot: - SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait'); - <waiting ...> -step s4_wakeup_s1_setup: - SELECT CASE WHEN - (SELECT pid FROM pg_stat_activity - WHERE wait_event_type = 'InjectionPoint' AND - wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL - THEN injection_points_wakeup('invalidate-catalog-snapshot-end') - END; - -case ----- - -(1 row) - -step s1_attach_invalidate_catalog_snapshot: <... completed> -injection_points_attach ------------------------ - -(1 row) - -step s5_noop: - <waiting ...> -step s3_start_create_index: - CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_duplicate ON test.tbl(i); - <waiting ...> -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_start_upsert: - INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); - <waiting ...> -step s5_noop: <... completed> -step s4_wakeup_define_index_before_set_valid: - SELECT injection_points_detach('define-index-before-set-valid'); - SELECT injection_points_wakeup('define-index-before-set-valid'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s2_start_upsert: - INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); - <waiting ...> -step s5_wakeup_s1_from_invalidate_catalog_snapshot: - SELECT injection_points_detach('invalidate-catalog-snapshot-end'); - SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s4_wakeup_s2: - SELECT injection_points_detach('exec-insert-before-insert-speculative'); - SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -step s4_wakeup_s1: - SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); - SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); - -injection_points_detach ------------------------ - -(1 row) - -injection_points_wakeup ------------------------ - -(1 row) - -s1: NOTICE: notice triggered for injection point pre-invalidate-catalog-snapshot-end -step s1_start_upsert: <... completed> -step s2_start_upsert: <... completed> -step s3_start_create_index: <... completed> diff --git a/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec b/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec index 1cbcaa6963f..34016b93bb0 100644 --- a/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec +++ b/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec @@ -27,7 +27,6 @@ setup { SELECT injection_points_set_local(); SELECT injection_points_attach('check-exclusion-or-unique-constraint-no-conflict', 'wait'); - SELECT injection_points_attach('pre-invalidate-catalog-snapshot-end', 'notice'); } step s1_attach_invalidate_catalog_snapshot { @@ -91,19 +90,43 @@ step s4_wakeup_define_index_before_set_valid } session s5 -step s5_noop -{ +setup { + CREATE OR REPLACE PROCEDURE test.wait_for_injection_point() + LANGUAGE plpgsql + AS $$ + DECLARE + v_waiting_pid INTEGER; + BEGIN + SELECT pid INTO v_waiting_pid + FROM pg_stat_activity + WHERE wait_event_type = 'InjectionPoint' + AND wait_event = 'invalidate-catalog-snapshot-end' + LIMIT 1; + + IF v_waiting_pid IS NOT NULL THEN + RETURN; + END IF; + + PERFORM pg_sleep(100); + + CALL test.wait_for_injection_point(); + END; + $$; } step s5_wakeup_s1_from_invalidate_catalog_snapshot { + CALL test.wait_for_injection_point(); SELECT injection_points_detach('invalidate-catalog-snapshot-end'); SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); } +teardown +{ + DROP PROCEDURE test.wait_for_injection_point; +} permutation s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup - s5_noop(s1_start_upsert notices 1) s3_start_create_index(s1_start_upsert, s2_start_upsert) s1_start_upsert s4_wakeup_define_index_before_set_valid diff --git a/src/test/modules/injection_points/specs/index-concurrently-upsert.spec b/src/test/modules/injection_points/specs/index-concurrently-upsert.spec index 2a6d888dcea..92fc0933e91 100644 --- a/src/test/modules/injection_points/specs/index-concurrently-upsert.spec +++ b/src/test/modules/injection_points/specs/index-concurrently-upsert.spec @@ -26,7 +26,6 @@ setup { SELECT injection_points_set_local(); SELECT injection_points_attach('check-exclusion-or-unique-constraint-no-conflict', 'wait'); - SELECT injection_points_attach('pre-invalidate-catalog-snapshot-end', 'notice'); } step s1_attach_invalidate_catalog_snapshot { @@ -34,7 +33,7 @@ step s1_attach_invalidate_catalog_snapshot } step s1_start_upsert { - INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); + INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now(); } session s2 @@ -68,8 +67,8 @@ step s4_wakeup_s1_setup { SELECT CASE WHEN (SELECT pid FROM pg_stat_activity - WHERE wait_event_type = 'InjectionPoint' AND - wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL + WHERE wait_event_type = 'InjectionPoint' AND + wait_event = 'invalidate-catalog-snapshot-end') IS NOT NULL THEN injection_points_wakeup('invalidate-catalog-snapshot-end') END; } @@ -90,19 +89,44 @@ step s4_wakeup_define_index_before_set_valid } session s5 -step s5_noop +setup { + CREATE OR REPLACE PROCEDURE test.wait_for_injection_point() + LANGUAGE plpgsql + AS $$ + DECLARE + v_waiting_pid INTEGER; + BEGIN + SELECT pid INTO v_waiting_pid + FROM pg_stat_activity + WHERE wait_event_type = 'InjectionPoint' + AND wait_event = 'invalidate-catalog-snapshot-end' + LIMIT 1; + + IF v_waiting_pid IS NOT NULL THEN + RETURN; + END IF; + + PERFORM pg_sleep(100); + + CALL test.wait_for_injection_point(); + END; + $$; } step s5_wakeup_s1_from_invalidate_catalog_snapshot { + CALL test.wait_for_injection_point(); SELECT injection_points_detach('invalidate-catalog-snapshot-end'); SELECT injection_points_wakeup('invalidate-catalog-snapshot-end'); } +teardown +{ + DROP PROCEDURE test.wait_for_injection_point; +} permutation s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup - s5_noop(s1_start_upsert notices 1) s3_start_create_index(s1_start_upsert, s2_start_upsert) s1_start_upsert s4_wakeup_define_index_before_set_valid -- 2.43.0
From af5e27d4150dd53d313122c02da7ce4d3c07f332 Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu <[email protected]> Date: Sun, 30 Nov 2025 16:48:55 +0100 Subject: [PATCH v16 1/3] Modify the ExecInitPartitionInfo function to consider partitioned indexes that are potentially processed by REINDEX CONCURRENTLY as arbiters as well. This is necessary to ensure that all concurrent transactions use the same set of arbiter indexes. --- src/backend/executor/execPartition.c | 127 +++++++++- src/test/modules/injection_points/Makefile | 3 +- ...eindex-concurrently-upsert-partitioned.out | 232 ++++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + ...index-concurrently-upsert-partitioned.spec | 96 ++++++++ 5 files changed, 446 insertions(+), 13 deletions(-) create mode 100644 src/test/modules/injection_points/expected/reindex-concurrently-upsert-partitioned.out create mode 100644 src/test/modules/injection_points/specs/reindex-concurrently-upsert-partitioned.spec diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 0dcce181f09..bd3527393ec 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -15,6 +15,7 @@ #include "access/table.h" #include "access/tableam.h" +#include "catalog/index.h" #include "catalog/partition.h" #include "executor/execPartition.h" #include "executor/executor.h" @@ -490,6 +491,62 @@ ExecFindPartition(ModifyTableState *mtstate, return rri; } +/* + * IsIndexCompatibleAsArbiter + * Checks if the indexes are identical in terms of being used + * as arbiters for the INSERT ON CONFLICT operation by comparing + * them to the provided arbiter index. + * + * Only indexes of the same relation are supported. + * + * Returns the true if indexes are compatible. + */ +static bool +IsIndexCompatibleAsArbiter(Relation arbiterIndexRelation, + IndexInfo *arbiterIndexInfo, + Relation indexRelation, + IndexInfo *indexInfo) +{ + int i; + + Assert(arbiterIndexRelation->rd_index->indrelid == indexRelation->rd_index->indrelid); + + if (arbiterIndexInfo->ii_Unique != indexInfo->ii_Unique) + return false; + + if (arbiterIndexInfo->ii_NullsNotDistinct != indexInfo->ii_NullsNotDistinct) + return false; + + /* and same number of key attributes */ + if (arbiterIndexInfo->ii_NumIndexKeyAttrs != indexInfo->ii_NumIndexKeyAttrs) + return false; + + /* No support currently for comparing exclusion indexes. */ + if (arbiterIndexInfo->ii_ExclusionOps != NULL || indexInfo->ii_ExclusionOps != NULL) + return false; + + for (i = 0; i < arbiterIndexInfo->ii_NumIndexKeyAttrs; i++) + { + if (arbiterIndexRelation->rd_indcollation[i] != indexRelation->rd_indcollation[i]) + return false; + + if (arbiterIndexRelation->rd_opfamily[i] != indexRelation->rd_opfamily[i]) + return false; + + if (arbiterIndexRelation->rd_index->indkey.values[i] != indexRelation->rd_index->indkey.values[i]) + return false; + } + + if (list_difference(RelationGetIndexExpressions(arbiterIndexRelation), + RelationGetIndexExpressions(indexRelation)) != NIL) + return false; + + if (list_difference(RelationGetIndexPredicate(arbiterIndexRelation), + RelationGetIndexPredicate(indexRelation)) != NIL) + return false; + return true; +} + /* * ExecInitPartitionInfo * Lock the partition and initialize ResultRelInfo. Also setup other @@ -690,7 +747,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, TupleDesc partrelDesc = RelationGetDescr(partrel); ExprContext *econtext = mtstate->ps.ps_ExprContext; ListCell *lc; - List *arbiterIndexes = NIL; + List *arbiterIndexes = NIL, + *arbiterIndexesOffset = NIL; + int additional_arbiters = 0; /* * If there is a list of arbiter indexes, map it to a list of indexes @@ -698,36 +757,80 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * list and searching for ancestry relationships to each index in the * ancestor table. */ - if (rootResultRelInfo->ri_onConflictArbiterIndexes != NIL) - { - List *childIdxs; + if (rootResultRelInfo->ri_onConflictArbiterIndexes != NIL) { + List *childIdxs, + *nonAncestorIdxOffset = NIL; childIdxs = RelationGetIndexList(leaf_part_rri->ri_RelationDesc); foreach(lc, childIdxs) { Oid childIdx = lfirst_oid(lc); + int i = foreach_current_index(lc); List *ancestors; - ListCell *lc2; ancestors = get_partition_ancestors(childIdx); - foreach(lc2, rootResultRelInfo->ri_onConflictArbiterIndexes) + if (ancestors) { - if (list_member_oid(ancestors, lfirst_oid(lc2))) - arbiterIndexes = lappend_oid(arbiterIndexes, childIdx); + foreach_oid(oid, rootResultRelInfo->ri_onConflictArbiterIndexes) + { + if (list_member_oid(ancestors, oid)) + { + arbiterIndexes = lappend_oid(arbiterIndexes, childIdx); + arbiterIndexesOffset = lappend_int(arbiterIndexesOffset, i); + } + } } + else /* No ancestor was found for that index. Save it for rechecking later. */ + nonAncestorIdxOffset = lappend_int(nonAncestorIdxOffset, i); + list_free(ancestors); } + + /* + * If any non-ancestor indexes are found, we need to compare them with other + * indexes of the relation that will be used as arbiters. This is necessary + * when a partitioned index is processed by REINDEX CONCURRENTLY. Both indexes + * must be considered as arbiters to ensure that all concurrent transactions + * use the same set of arbiters. + */ + if (nonAncestorIdxOffset && arbiterIndexesOffset) + { + foreach_int(childIdxOffset, nonAncestorIdxOffset) + { + Relation nonAncestorIndexRelation = leaf_part_rri->ri_IndexRelationDescs[childIdxOffset]; + IndexInfo *nonAncestorIndexInfo = leaf_part_rri->ri_IndexRelationInfo[childIdxOffset]; + Assert(!list_member_oid(arbiterIndexes, nonAncestorIndexRelation->rd_index->indexrelid)); + + /* It is too early to use non-ready indexes as arbiters */ + if (!nonAncestorIndexInfo->ii_ReadyForInserts) + continue; + + foreach_int(arbiterIdxOffset, arbiterIndexesOffset) + { + Relation arbiterIndexRelation = leaf_part_rri->ri_IndexRelationDescs[arbiterIdxOffset]; + IndexInfo *arbiterIndexInfo = leaf_part_rri->ri_IndexRelationInfo[arbiterIdxOffset]; + + /* If non-ancestor index are compatible to arbiter - use it as arbiter too. */ + if (IsIndexCompatibleAsArbiter(arbiterIndexRelation, arbiterIndexInfo, + nonAncestorIndexRelation, nonAncestorIndexInfo)) + { + arbiterIndexes = lappend_oid(arbiterIndexes, + nonAncestorIndexRelation->rd_index->indexrelid); + additional_arbiters++; + } + } + } + } + list_free(nonAncestorIdxOffset); } /* * If the resulting lists are of inequal length, something is wrong. - * XXX This may happen because we don't match the lists correctly when - * a partitioned index is being processed by REINDEX CONCURRENTLY. - * FIXME later. + * But we need to consider additional arbiter indexes also. */ if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes) != - list_length(arbiterIndexes)) + list_length(arbiterIndexes) - additional_arbiters) elog(ERROR, "invalid arbiter index list"); leaf_part_rri->ri_onConflictArbiterIndexes = arbiterIndexes; diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 7b3c0c4b716..e10475d2665 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -19,7 +19,8 @@ ISOLATION = basic \ syscache-update-pruned \ index-concurrently-upsert \ reindex-concurrently-upsert \ - index-concurrently-upsert-predicate + index-concurrently-upsert-predicate \ + reindex-concurrently-upsert-partitioned TAP_TESTS = 1 diff --git a/src/test/modules/injection_points/expected/reindex-concurrently-upsert-partitioned.out b/src/test/modules/injection_points/expected/reindex-concurrently-upsert-partitioned.out new file mode 100644 index 00000000000..588ffbd57a5 --- /dev/null +++ b/src/test/modules/injection_points/expected/reindex-concurrently-upsert-partitioned.out @@ -0,0 +1,232 @@ +Parsed test spec with 4 sessions + +starting permutation: s3_setup_wait_before_set_dead s3_start_reindex s1_start_upsert s4_wakeup_to_set_dead s2_start_upsert s4_wakeup_s1 s4_wakeup_s2 +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_set_local +-------------------------- + +(1 row) + +step s3_setup_wait_before_set_dead: + SELECT injection_points_attach('reindex-relation-concurrently-before-set-dead', 'wait'); + +injection_points_attach +----------------------- + +(1 row) + +step s3_start_reindex: REINDEX INDEX CONCURRENTLY test.tbl_partition_pkey; <waiting ...> +step s1_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s4_wakeup_to_set_dead: + SELECT injection_points_detach('reindex-relation-concurrently-before-set-dead'); + SELECT injection_points_wakeup('reindex-relation-concurrently-before-set-dead'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s4_wakeup_s1: + SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); + SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s1_start_upsert: <... completed> +step s4_wakeup_s2: + SELECT injection_points_detach('exec-insert-before-insert-speculative'); + SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_start_upsert: <... completed> +step s3_start_reindex: <... completed> + +starting permutation: s3_setup_wait_before_swap s3_start_reindex s1_start_upsert s4_wakeup_to_swap s2_start_upsert s4_wakeup_s2 s4_wakeup_s1 +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_set_local +-------------------------- + +(1 row) + +step s3_setup_wait_before_swap: + SELECT injection_points_attach('reindex-relation-concurrently-before-swap', 'wait'); + +injection_points_attach +----------------------- + +(1 row) + +step s3_start_reindex: REINDEX INDEX CONCURRENTLY test.tbl_partition_pkey; <waiting ...> +step s1_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s4_wakeup_to_swap: + SELECT injection_points_detach('reindex-relation-concurrently-before-swap'); + SELECT injection_points_wakeup('reindex-relation-concurrently-before-swap'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s4_wakeup_s2: + SELECT injection_points_detach('exec-insert-before-insert-speculative'); + SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s4_wakeup_s1: + SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); + SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s1_start_upsert: <... completed> +step s2_start_upsert: <... completed> +step s3_start_reindex: <... completed> + +starting permutation: s3_setup_wait_before_set_dead s3_start_reindex s1_start_upsert s2_start_upsert s4_wakeup_s1 s4_wakeup_to_set_dead s4_wakeup_s2 +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_set_local +-------------------------- + +(1 row) + +step s3_setup_wait_before_set_dead: + SELECT injection_points_attach('reindex-relation-concurrently-before-set-dead', 'wait'); + +injection_points_attach +----------------------- + +(1 row) + +step s3_start_reindex: REINDEX INDEX CONCURRENTLY test.tbl_partition_pkey; <waiting ...> +step s1_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s2_start_upsert: + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); + <waiting ...> +step s4_wakeup_s1: + SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); + SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s1_start_upsert: <... completed> +step s4_wakeup_to_set_dead: + SELECT injection_points_detach('reindex-relation-concurrently-before-set-dead'); + SELECT injection_points_wakeup('reindex-relation-concurrently-before-set-dead'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s4_wakeup_s2: + SELECT injection_points_detach('exec-insert-before-insert-speculative'); + SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_start_upsert: <... completed> +step s3_start_reindex: <... completed> diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 485b483e3ca..3e06220fe31 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -51,6 +51,7 @@ tests += { 'index-concurrently-upsert', 'reindex-concurrently-upsert', 'index-concurrently-upsert-predicate', + 'reindex-concurrently-upsert-partitioned' ], 'runningcheck': false, # see syscache-update-pruned # Some tests wait for all snapshots, so avoid parallel execution diff --git a/src/test/modules/injection_points/specs/reindex-concurrently-upsert-partitioned.spec b/src/test/modules/injection_points/specs/reindex-concurrently-upsert-partitioned.spec new file mode 100644 index 00000000000..c8a8eb9cca5 --- /dev/null +++ b/src/test/modules/injection_points/specs/reindex-concurrently-upsert-partitioned.spec @@ -0,0 +1,96 @@ +# Test race conditions involving: +# - s1: UPSERT a tuple +# - s2: UPSERT the same tuple +# - s3: REINDEX concurrent primary key index +# - s4: operations with injection points + +setup +{ + CREATE EXTENSION injection_points; + CREATE SCHEMA test; + CREATE TABLE test.tbl(i int primary key, updated_at timestamp) PARTITION BY RANGE (i); + CREATE TABLE test.tbl_partition PARTITION OF test.tbl + FOR VALUES FROM (0) TO (10000) + WITH (parallel_workers = 0); +} + +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', 'wait'); +} +step s1_start_upsert { + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); +} + +session s2 +setup { + SELECT injection_points_set_local(); + SELECT injection_points_attach('exec-insert-before-insert-speculative', 'wait'); +} +step s2_start_upsert { + INSERT INTO test.tbl VALUES(13,now()) on conflict(i) do update set updated_at = now(); +} + +session s3 +setup { + SELECT injection_points_set_local(); +} +step s3_setup_wait_before_set_dead { + SELECT injection_points_attach('reindex-relation-concurrently-before-set-dead', 'wait'); +} +step s3_setup_wait_before_swap { + SELECT injection_points_attach('reindex-relation-concurrently-before-swap', 'wait'); +} +step s3_start_reindex { REINDEX INDEX CONCURRENTLY test.tbl_partition_pkey; } + +session s4 +step s4_wakeup_to_swap { + SELECT injection_points_detach('reindex-relation-concurrently-before-swap'); + SELECT injection_points_wakeup('reindex-relation-concurrently-before-swap'); +} +step s4_wakeup_s1 { + SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict'); + SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict'); +} +step s4_wakeup_s2 { + SELECT injection_points_detach('exec-insert-before-insert-speculative'); + SELECT injection_points_wakeup('exec-insert-before-insert-speculative'); +} +step s4_wakeup_to_set_dead { + SELECT injection_points_detach('reindex-relation-concurrently-before-set-dead'); + SELECT injection_points_wakeup('reindex-relation-concurrently-before-set-dead'); +} + +permutation + s3_setup_wait_before_set_dead + s3_start_reindex(s1_start_upsert, s2_start_upsert) + s1_start_upsert + s4_wakeup_to_set_dead + s2_start_upsert(s1_start_upsert) + s4_wakeup_s1 + s4_wakeup_s2 + +permutation + s3_setup_wait_before_swap + s3_start_reindex(s1_start_upsert, s2_start_upsert) + s1_start_upsert + s4_wakeup_to_swap + s2_start_upsert(s1_start_upsert) + s4_wakeup_s2 + s4_wakeup_s1 + +permutation + s3_setup_wait_before_set_dead + s3_start_reindex(s1_start_upsert, s2_start_upsert) + s1_start_upsert + s2_start_upsert(s1_start_upsert) + s4_wakeup_s1 + s4_wakeup_to_set_dead + s4_wakeup_s2 \ No newline at end of file -- 2.43.0
