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

Commit8a54e12

Browse files
committed
Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions.
In a cluster having used CREATE INDEX CONCURRENTLY while having enabledprepared transactions, queries that use the resulting index can silentlyfail to find rows. Fix this for future CREATE INDEX CONCURRENTLY bymaking it wait for prepared transactions like it waits for ordinarytransactions. This expands the VirtualTransactionId structure domain toadmit prepared transactions. It may be necessary to reindex to recoverfrom past occurrences. Back-patch to 9.5 (all supported versions).Andrey Borodin, reviewed (in earlier versions) by Tom Lane and MichaelPaquier.Discussion:https://postgr.es/m/2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru
1 parentf77717b commit8a54e12

File tree

7 files changed

+98
-37
lines changed

7 files changed

+98
-37
lines changed

‎src/backend/storage/lmgr/lmgr.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -904,8 +904,7 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
904904

905905
/*
906906
* Note: GetLockConflicts() never reports our own xid, hence we need not
907-
* check for that. Also, prepared xacts are not reported, which is fine
908-
* since they certainly aren't going to do anything anymore.
907+
* check for that. Also, prepared xacts are reported and awaited.
909908
*/
910909

911910
/* Finally wait for each such transaction to complete */

‎src/backend/storage/lmgr/lock.c

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2903,9 +2903,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
29032903
* so use of this function has to be thought about carefully.
29042904
*
29052905
* Note we never include the current xact's vxid in the result array,
2906-
* since an xact never blocks itself. Also, prepared transactions are
2907-
* ignored, which is a bit more debatable but is appropriate for current
2908-
* uses of the result.
2906+
* since an xact never blocks itself.
29092907
*/
29102908
VirtualTransactionId*
29112909
GetLockConflicts(constLOCKTAG*locktag,LOCKMODElockmode,int*countp)
@@ -2930,19 +2928,21 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
29302928

29312929
/*
29322930
* Allocate memory to store results, and fill with InvalidVXID. We only
2933-
* need enough space for MaxBackends +a terminator, since prepared xacts
2934-
*don't count.InHotStandby allocate once in TopMemoryContext.
2931+
* need enough space for MaxBackends +max_prepared_xacts + a terminator.
2932+
* InHotStandby allocate once in TopMemoryContext.
29352933
*/
29362934
if (InHotStandby)
29372935
{
29382936
if (vxids==NULL)
29392937
vxids= (VirtualTransactionId*)
29402938
MemoryContextAlloc(TopMemoryContext,
2941-
sizeof(VirtualTransactionId)* (MaxBackends+1));
2939+
sizeof(VirtualTransactionId)*
2940+
(MaxBackends+max_prepared_xacts+1));
29422941
}
29432942
else
29442943
vxids= (VirtualTransactionId*)
2945-
palloc0(sizeof(VirtualTransactionId)* (MaxBackends+1));
2944+
palloc0(sizeof(VirtualTransactionId)*
2945+
(MaxBackends+max_prepared_xacts+1));
29462946

29472947
/* Compute hash code and partition lock, and look up conflicting modes. */
29482948
hashcode=LockTagHashCode(locktag);
@@ -3017,13 +3017,9 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
30173017
/* Conflict! */
30183018
GET_VXID_FROM_PGPROC(vxid,*proc);
30193019

3020-
/*
3021-
* If we see an invalid VXID, then either the xact has already
3022-
* committed (or aborted), or it's a prepared xact. In either
3023-
* case we may ignore it.
3024-
*/
30253020
if (VirtualTransactionIdIsValid(vxid))
30263021
vxids[count++]=vxid;
3022+
/* else, xact already committed or aborted */
30273023

30283024
/* No need to examine remaining slots. */
30293025
break;
@@ -3082,11 +3078,6 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
30823078

30833079
GET_VXID_FROM_PGPROC(vxid,*proc);
30843080

3085-
/*
3086-
* If we see an invalid VXID, then either the xact has already
3087-
* committed (or aborted), or it's a prepared xact. In either
3088-
* case we may ignore it.
3089-
*/
30903081
if (VirtualTransactionIdIsValid(vxid))
30913082
{
30923083
inti;
@@ -3098,6 +3089,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
30983089
if (i >=fast_count)
30993090
vxids[count++]=vxid;
31003091
}
3092+
/* else, xact already committed or aborted */
31013093
}
31023094
}
31033095

@@ -3107,7 +3099,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
31073099

31083100
LWLockRelease(partitionLock);
31093101

3110-
if (count>MaxBackends)/* should never happen */
3102+
if (count>MaxBackends+max_prepared_xacts)/* should never happen */
31113103
elog(PANIC,"too many conflicting locks found");
31123104

31133105
vxids[count].backendId=InvalidBackendId;
@@ -4464,6 +4456,21 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
44644456

44654457
Assert(VirtualTransactionIdIsValid(vxid));
44664458

4459+
if (VirtualTransactionIdIsPreparedXact(vxid))
4460+
{
4461+
LockAcquireResultlar;
4462+
4463+
/*
4464+
* Prepared transactions don't hold vxid locks. The
4465+
* LocalTransactionId is always a normal, locked XID.
4466+
*/
4467+
SET_LOCKTAG_TRANSACTION(tag,vxid.localTransactionId);
4468+
lar=LockAcquire(&tag,ShareLock, false, !wait);
4469+
if (lar!=LOCKACQUIRE_NOT_AVAIL)
4470+
LockRelease(&tag,ShareLock, false);
4471+
returnlar!=LOCKACQUIRE_NOT_AVAIL;
4472+
}
4473+
44674474
SET_LOCKTAG_VIRTUALTRANSACTION(tag,vxid);
44684475

44694476
/*

‎src/include/storage/lock.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ extern bool Debug_deadlocks;
4646

4747
/*
4848
* Top-level transactions are identified by VirtualTransactionIDs comprising
49-
*the BackendId of the backend running the xact, plus a locally-assigned
50-
* LocalTransactionId. These are guaranteed unique over the short term,
51-
* but will be reused after a database restart; hence they should never
52-
* be stored on disk.
49+
*PGPROC fields backendId and lxid. For prepared transactions, the
50+
* LocalTransactionId is an ordinary XID. These are guaranteed unique over
51+
*the short term,but will be reused after a database restart or XID
52+
*wraparound; hence they should neverbe stored on disk.
5353
*
5454
* Note that struct VirtualTransactionId can not be assumed to be atomically
5555
* assignable as a whole. However, type LocalTransactionId is assumed to
@@ -61,15 +61,16 @@ extern bool Debug_deadlocks;
6161
*/
6262
typedefstruct
6363
{
64-
BackendIdbackendId;/*determined at backend startup */
65-
LocalTransactionIdlocalTransactionId;/*backend-local transaction id */
64+
BackendIdbackendId;/*backendId from PGPROC */
65+
LocalTransactionIdlocalTransactionId;/*lxid from PGPROC */
6666
}VirtualTransactionId;
6767

6868
#defineInvalidLocalTransactionId0
6969
#defineLocalTransactionIdIsValid(lxid) ((lxid) != InvalidLocalTransactionId)
7070
#defineVirtualTransactionIdIsValid(vxid) \
71-
(((vxid).backendId != InvalidBackendId) && \
72-
LocalTransactionIdIsValid((vxid).localTransactionId))
71+
(LocalTransactionIdIsValid((vxid).localTransactionId))
72+
#defineVirtualTransactionIdIsPreparedXact(vxid) \
73+
((vxid).backendId == InvalidBackendId)
7374
#defineVirtualTransactionIdEquals(vxid1,vxid2) \
7475
((vxid1).backendId == (vxid2).backendId && \
7576
(vxid1).localTransactionId == (vxid2).localTransactionId)

‎src/test/isolation/Makefile

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,11 @@ installcheck: all
6262
check: all
6363
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule
6464

65-
# Versions of the check tests that include the prepared-transactions test
66-
# It only makes sense to run these if set up to use prepared transactions,
67-
# via TEMP_CONFIG for the check case, or via the postgresql.conf for the
68-
# installcheck case.
65+
# Non-default tests. It only makes sense to run these if set up to use
66+
# prepared transactions, via TEMP_CONFIG for the check case, or via the
67+
# postgresql.conf for the installcheck case.
6968
installcheck-prepared-txns: all temp-install
70-
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions
69+
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic
7170

7271
check-prepared-txns: all temp-install
73-
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions
72+
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic

‎src/test/isolation/README

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ you can do something like
2323
./pg_isolation_regress fk-contention fk-deadlock
2424
(look into the specs/ subdirectory to see the available tests).
2525

26-
The prepared-transactions test requiresthe server's
27-
max_prepared_transactions parameter to beset to at least 3; thereforeit
28-
is not run by default. To include it inthe test run, use
26+
Certain tests requirethe server's max_prepared_transactions parameter to be
27+
set to at least 3; thereforethey are not run by default. To include them in
28+
the test run, use
2929
make check-prepared-txns
3030
or
3131
make installcheck-prepared-txns
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: w1 p1 cic2 c1 r2
4+
step w1: BEGIN; INSERT INTO cic_test VALUES (1);
5+
step p1: PREPARE TRANSACTION 's1';
6+
step cic2:
7+
CREATE INDEX CONCURRENTLY on cic_test(a);
8+
9+
ERROR: canceling statement due to lock timeout
10+
step c1: COMMIT PREPARED 's1';
11+
step r2:
12+
SET enable_seqscan to off;
13+
SET enable_bitmapscan to off;
14+
SELECT * FROM cic_test WHERE a = 1;
15+
16+
a
17+
18+
1
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# This test verifies that CREATE INDEX CONCURRENTLY interacts with prepared
2+
# transactions correctly.
3+
setup
4+
{
5+
CREATETABLEcic_test(aint);
6+
}
7+
8+
teardown
9+
{
10+
DROPTABLEcic_test;
11+
}
12+
13+
14+
# Sessions for CREATE INDEX CONCURRENTLY test
15+
session"s1"
16+
step"w1"{BEGIN;INSERTINTOcic_testVALUES(1);}
17+
step"p1"{PREPARETRANSACTION's1';}
18+
step"c1"{COMMITPREPARED's1';}
19+
20+
session"s2"
21+
# The isolation tester never recognizes that a lock of s1 blocks s2, because a
22+
# prepared transaction's locks have no pid associated. While there's a slight
23+
# chance of timeout while waiting for an autovacuum-held lock, that wouldn't
24+
# change the output. Hence, no timeout is too short.
25+
setup{SETlock_timeout=10;}
26+
step"cic2"
27+
{
28+
CREATEINDEXCONCURRENTLYoncic_test(a);
29+
}
30+
step"r2"
31+
{
32+
SETenable_seqscantooff;
33+
SETenable_bitmapscantooff;
34+
SELECT *FROMcic_testWHEREa=1;
35+
}
36+
37+
permutation"w1""p1""cic2""c1""r2"

0 commit comments

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