Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbc32a12

Browse files
committed
Fix infer_arbiter_index during concurrent index operations
Previously, we would only consider indexes marked indisvalid as usablefor INSERT ON CONFLICT. But that's problematic during CREATE INDEXCONCURRENTLY and REINDEX CONCURRENTLY, because concurrent transactionswould end up with inconsistents lists of inferred indexes, leading todeadlocks and spurious errors about unique key violations (because twotransactions are operating on different indexes for the speculativeinsertion tokens). Change this function to return indexes even ifinvalid. This fixes the spurious errors and deadlocks.Because such indexes might not be complete, we still need uniqueness tobe verified in a different way. We do that by requiring that at leastone index marked valid is part of the set of indexes returned. It isthat index that is going to help ensure that the inserted tuple isindeed unique.This does not fix similar problems occurring with partitioned tables orwith named constraints. These problems will be fixed in follow-upcommits.We have no user report of this problem, even though it exists in allbranches. Because of that and given that the fix is somewhat tricky, Idecided not to backpatch for now.Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com>Reviewed-by: Michael Paquier <michael@paquier.xyz>Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>Discussion:https://postgr.es/m/CANtu0ogv+6wqRzPK241jik4U95s1pW3MCZ3rX5ZqbFdUysz7Qw@mail.gmail.com
1 parente429c3c commitbc32a12

File tree

14 files changed

+749
-8
lines changed

14 files changed

+749
-8
lines changed

‎src/backend/commands/indexcmds.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,7 @@ DefineIndex(Oid tableId,
17891789
* before the reference snap was taken, we have to wait out any
17901790
* transactions that might have older snapshots.
17911791
*/
1792+
INJECTION_POINT("define-index-before-set-valid",NULL);
17921793
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
17931794
PROGRESS_CREATEIDX_PHASE_WAIT_3);
17941795
WaitForOlderSnapshots(limitXmin, true);
@@ -4229,6 +4230,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
42294230
* indexes with the correct names.
42304231
*/
42314232

4233+
INJECTION_POINT("reindex-relation-concurrently-before-swap",NULL);
42324234
StartTransactionCommand();
42334235

42344236
/*
@@ -4307,6 +4309,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
43074309
* index_drop() for more details.
43084310
*/
43094311

4312+
INJECTION_POINT("reindex-relation-concurrently-before-set-dead",NULL);
43104313
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
43114314
PROGRESS_CREATEIDX_PHASE_WAIT_4);
43124315
WaitForLockersMultiple(lockTags,AccessExclusiveLock, true);

‎src/backend/executor/execIndexing.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
#include"executor/executor.h"
115115
#include"nodes/nodeFuncs.h"
116116
#include"storage/lmgr.h"
117+
#include"utils/injection_point.h"
117118
#include"utils/multirangetypes.h"
118119
#include"utils/rangetypes.h"
119120
#include"utils/snapmgr.h"
@@ -943,6 +944,13 @@ check_exclusion_or_unique_constraint(Relation heap, Relation index,
943944

944945
ExecDropSingleTupleTableSlot(existing_slot);
945946

947+
#ifdefUSE_INJECTION_POINTS
948+
if (conflict)
949+
INJECTION_POINT("check-exclusion-or-unique-constraint-conflict",NULL);
950+
else
951+
INJECTION_POINT("check-exclusion-or-unique-constraint-no-conflict",NULL);
952+
#endif
953+
946954
return !conflict;
947955
}
948956

‎src/backend/executor/execPartition.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -722,8 +722,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
722722

723723
/*
724724
* If the resulting lists are of inequal length, something is wrong.
725-
* (This shouldn't happen, since arbiter index selection should not
726-
* pick up an invalid index.)
725+
* XXX This may happen because we don't match the lists correctly when
726+
* a partitioned index is being processed by REINDEX CONCURRENTLY.
727+
* FIXME later.
727728
*/
728729
if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes)!=
729730
list_length(arbiterIndexes))

‎src/backend/executor/nodeModifyTable.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
#include"storage/lmgr.h"
6969
#include"utils/builtins.h"
7070
#include"utils/datum.h"
71+
#include"utils/injection_point.h"
7172
#include"utils/rel.h"
7273
#include"utils/snapmgr.h"
7374

@@ -1186,6 +1187,7 @@ ExecInsert(ModifyTableContext *context,
11861187
* if we're going to go ahead with the insertion, instead of
11871188
* waiting for the whole transaction to complete.
11881189
*/
1190+
INJECTION_POINT("exec-insert-before-insert-speculative",NULL);
11891191
specToken=SpeculativeInsertionLockAcquire(GetCurrentTransactionId());
11901192

11911193
/* insert the tuple, with the speculative token */

‎src/backend/optimizer/util/plancat.c‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,7 @@ infer_arbiter_indexes(PlannerInfo *root)
814814

815815
/* Results */
816816
List*results=NIL;
817+
boolfoundValid= false;
817818

818819
/*
819820
* Quickly return NIL for ON CONFLICT DO NOTHING without an inference
@@ -907,7 +908,22 @@ infer_arbiter_indexes(PlannerInfo *root)
907908
idxRel=index_open(indexoid,rte->rellockmode);
908909
idxForm=idxRel->rd_index;
909910

910-
if (!idxForm->indisvalid)
911+
/*
912+
* Ignore indexes that aren't indisready, because we cannot trust
913+
* their catalog structure yet. However, if any indexes are marked
914+
* indisready but not yet indisvalid, we still consider them, because
915+
* they might turn valid while we're running. Doing it this way
916+
* allows a concurrent transaction with a slightly later catalog
917+
* snapshot infer the same set of indexes, which is critical to
918+
* prevent spurious 'duplicate key' errors.
919+
*
920+
* However, another critical aspect is that a unique index that isn't
921+
* yet marked indisvalid=true might not be complete yet, meaning it
922+
* wouldn't detect possible duplicate rows. In order to prevent false
923+
* negatives, we require that we include in the set of inferred
924+
* indexes at least one index that is marked valid.
925+
*/
926+
if (!idxForm->indisready)
911927
gotonext;
912928

913929
/*
@@ -929,10 +945,9 @@ infer_arbiter_indexes(PlannerInfo *root)
929945
errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints")));
930946

931947
results=lappend_oid(results,idxForm->indexrelid);
932-
list_free(indexList);
948+
foundValid |=idxForm->indisvalid;
933949
index_close(idxRel,NoLock);
934-
table_close(relation,NoLock);
935-
returnresults;
950+
break;
936951
}
937952
elseif (indexOidFromConstraint!=InvalidOid)
938953
{
@@ -1033,14 +1048,16 @@ infer_arbiter_indexes(PlannerInfo *root)
10331048
gotonext;
10341049

10351050
results=lappend_oid(results,idxForm->indexrelid);
1051+
foundValid |=idxForm->indisvalid;
10361052
next:
10371053
index_close(idxRel,NoLock);
10381054
}
10391055

10401056
list_free(indexList);
10411057
table_close(relation,NoLock);
10421058

1043-
if (results==NIL)
1059+
/* We require at least one indisvalid index */
1060+
if (results==NIL|| !foundValid)
10441061
ereport(ERROR,
10451062
(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
10461063
errmsg("there is no unique or exclusion constraint matching the ON CONFLICT specification")));

‎src/backend/utils/time/snapmgr.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
#include"storage/proc.h"
120120
#include"storage/procarray.h"
121121
#include"utils/builtins.h"
122+
#include"utils/injection_point.h"
122123
#include"utils/memutils.h"
123124
#include"utils/resowner.h"
124125
#include"utils/snapmgr.h"
@@ -458,6 +459,7 @@ InvalidateCatalogSnapshot(void)
458459
pairingheap_remove(&RegisteredSnapshots,&CatalogSnapshot->ph_node);
459460
CatalogSnapshot=NULL;
460461
SnapshotResetXmin();
462+
INJECTION_POINT("invalidate-catalog-snapshot-end",NULL);
461463
}
462464
}
463465

‎src/test/modules/injection_points/Makefile‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@ PGFILEDESC = "injection_points - facility for injection points"
1414
REGRESS = injection_points hashagg reindex_conc vacuum
1515
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
1616

17-
ISOLATION = basic inplace syscache-update-pruned
17+
ISOLATION = basic\
18+
inplace\
19+
syscache-update-pruned\
20+
index-concurrently-upsert\
21+
reindex-concurrently-upsert\
22+
index-concurrently-upsert-predicate
1823

1924
TAP_TESTS = 1
2025

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
Parsed test spec with 4 sessions
2+
3+
starting permutation: s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s4_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
4+
injection_points_attach
5+
-----------------------
6+
7+
(1 row)
8+
9+
injection_points_attach
10+
-----------------------
11+
12+
(1 row)
13+
14+
injection_points_attach
15+
-----------------------
16+
17+
(1 row)
18+
19+
step s3_start_create_index:
20+
CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_special_duplicate ON test.tbl(abs(i)) WHERE i < 10000;
21+
<waiting ...>
22+
step s1_start_upsert:
23+
INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now();
24+
<waiting ...>
25+
step s4_wakeup_define_index_before_set_valid:
26+
SELECT injection_points_detach('define-index-before-set-valid');
27+
SELECT injection_points_wakeup('define-index-before-set-valid');
28+
29+
injection_points_detach
30+
-----------------------
31+
32+
(1 row)
33+
34+
injection_points_wakeup
35+
-----------------------
36+
37+
(1 row)
38+
39+
step s2_start_upsert:
40+
INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now();
41+
<waiting ...>
42+
step s4_wakeup_s1_from_invalidate_catalog_snapshot:
43+
SELECT injection_points_detach('invalidate-catalog-snapshot-end');
44+
SELECT injection_points_wakeup('invalidate-catalog-snapshot-end');
45+
46+
injection_points_detach
47+
-----------------------
48+
49+
(1 row)
50+
51+
injection_points_wakeup
52+
-----------------------
53+
54+
(1 row)
55+
56+
step s4_wakeup_s2:
57+
SELECT injection_points_detach('exec-insert-before-insert-speculative');
58+
SELECT injection_points_wakeup('exec-insert-before-insert-speculative');
59+
60+
injection_points_detach
61+
-----------------------
62+
63+
(1 row)
64+
65+
injection_points_wakeup
66+
-----------------------
67+
68+
(1 row)
69+
70+
step s4_wakeup_s1:
71+
SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
72+
SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict');
73+
74+
injection_points_detach
75+
-----------------------
76+
77+
(1 row)
78+
79+
injection_points_wakeup
80+
-----------------------
81+
82+
(1 row)
83+
84+
step s1_start_upsert: <... completed>
85+
step s2_start_upsert: <... completed>
86+
step s3_start_create_index: <... completed>
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
Parsed test spec with 4 sessions
2+
3+
starting permutation: s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s4_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
4+
injection_points_attach
5+
-----------------------
6+
7+
(1 row)
8+
9+
injection_points_attach
10+
-----------------------
11+
12+
(1 row)
13+
14+
injection_points_attach
15+
-----------------------
16+
17+
(1 row)
18+
19+
step s3_start_create_index:
20+
CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_duplicate ON test.tbl(i);
21+
<waiting ...>
22+
step s1_start_upsert:
23+
INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now();
24+
<waiting ...>
25+
step s4_wakeup_define_index_before_set_valid:
26+
SELECT injection_points_detach('define-index-before-set-valid');
27+
SELECT injection_points_wakeup('define-index-before-set-valid');
28+
29+
injection_points_detach
30+
-----------------------
31+
32+
(1 row)
33+
34+
injection_points_wakeup
35+
-----------------------
36+
37+
(1 row)
38+
39+
step s2_start_upsert:
40+
INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now();
41+
<waiting ...>
42+
step s4_wakeup_s1_from_invalidate_catalog_snapshot:
43+
SELECT injection_points_detach('invalidate-catalog-snapshot-end');
44+
SELECT injection_points_wakeup('invalidate-catalog-snapshot-end');
45+
46+
injection_points_detach
47+
-----------------------
48+
49+
(1 row)
50+
51+
injection_points_wakeup
52+
-----------------------
53+
54+
(1 row)
55+
56+
step s4_wakeup_s2:
57+
SELECT injection_points_detach('exec-insert-before-insert-speculative');
58+
SELECT injection_points_wakeup('exec-insert-before-insert-speculative');
59+
60+
injection_points_detach
61+
-----------------------
62+
63+
(1 row)
64+
65+
injection_points_wakeup
66+
-----------------------
67+
68+
(1 row)
69+
70+
step s4_wakeup_s1:
71+
SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
72+
SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict');
73+
74+
injection_points_detach
75+
-----------------------
76+
77+
(1 row)
78+
79+
injection_points_wakeup
80+
-----------------------
81+
82+
(1 row)
83+
84+
step s1_start_upsert: <... completed>
85+
step s2_start_upsert: <... completed>
86+
step s3_start_create_index: <... completed>

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp