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

Commit77a0e14

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 parentd777e27 commit77a0e14

File tree

7 files changed

+100
-38
lines changed

7 files changed

+100
-38
lines changed

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

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

812812
/*
813813
* Note: GetLockConflicts() never reports our own xid, hence we need not
814-
* check for that. Also, prepared xacts are not reported, which is fine
815-
* since they certainly aren't going to do anything anymore.
814+
* check for that. Also, prepared xacts are reported and awaited.
816815
*/
817816

818817
/* 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
@@ -2705,9 +2705,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
27052705
* so use of this function has to be thought about carefully.
27062706
*
27072707
* Note we never include the current xact's vxid in the result array,
2708-
* since an xact never blocks itself. Also, prepared transactions are
2709-
* ignored, which is a bit more debatable but is appropriate for current
2710-
* uses of the result.
2708+
* since an xact never blocks itself.
27112709
*/
27122710
VirtualTransactionId*
27132711
GetLockConflicts(constLOCKTAG*locktag,LOCKMODElockmode)
@@ -2732,19 +2730,21 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
27322730

27332731
/*
27342732
* Allocate memory to store results, and fill with InvalidVXID. We only
2735-
* need enough space for MaxBackends +a terminator, since prepared xacts
2736-
*don't count.InHotStandby allocate once in TopMemoryContext.
2733+
* need enough space for MaxBackends +max_prepared_xacts + a terminator.
2734+
* InHotStandby allocate once in TopMemoryContext.
27372735
*/
27382736
if (InHotStandby)
27392737
{
27402738
if (vxids==NULL)
27412739
vxids= (VirtualTransactionId*)
27422740
MemoryContextAlloc(TopMemoryContext,
2743-
sizeof(VirtualTransactionId)* (MaxBackends+1));
2741+
sizeof(VirtualTransactionId)*
2742+
(MaxBackends+max_prepared_xacts+1));
27442743
}
27452744
else
27462745
vxids= (VirtualTransactionId*)
2747-
palloc0(sizeof(VirtualTransactionId)* (MaxBackends+1));
2746+
palloc0(sizeof(VirtualTransactionId)*
2747+
(MaxBackends+max_prepared_xacts+1));
27482748

27492749
/* Compute hash code and partition lock, and look up conflicting modes. */
27502750
hashcode=LockTagHashCode(locktag);
@@ -2819,13 +2819,9 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
28192819
/* Conflict! */
28202820
GET_VXID_FROM_PGPROC(vxid,*proc);
28212821

2822-
/*
2823-
* If we see an invalid VXID, then either the xact has already
2824-
* committed (or aborted), or it's a prepared xact. In either
2825-
* case we may ignore it.
2826-
*/
28272822
if (VirtualTransactionIdIsValid(vxid))
28282823
vxids[count++]=vxid;
2824+
/* else, xact already committed or aborted */
28292825

28302826
/* No need to examine remaining slots. */
28312827
break;
@@ -2882,11 +2878,6 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
28822878

28832879
GET_VXID_FROM_PGPROC(vxid,*proc);
28842880

2885-
/*
2886-
* If we see an invalid VXID, then either the xact has already
2887-
* committed (or aborted), or it's a prepared xact. In either
2888-
* case we may ignore it.
2889-
*/
28902881
if (VirtualTransactionIdIsValid(vxid))
28912882
{
28922883
inti;
@@ -2898,6 +2889,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
28982889
if (i >=fast_count)
28992890
vxids[count++]=vxid;
29002891
}
2892+
/* else, xact already committed or aborted */
29012893
}
29022894
}
29032895

@@ -2907,7 +2899,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
29072899

29082900
LWLockRelease(partitionLock);
29092901

2910-
if (count>MaxBackends)/* should never happen */
2902+
if (count>MaxBackends+max_prepared_xacts)/* should never happen */
29112903
elog(PANIC,"too many conflicting locks found");
29122904

29132905
vxids[count].backendId=InvalidBackendId;
@@ -4052,6 +4044,21 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
40524044

40534045
Assert(VirtualTransactionIdIsValid(vxid));
40544046

4047+
if (VirtualTransactionIdIsPreparedXact(vxid))
4048+
{
4049+
LockAcquireResultlar;
4050+
4051+
/*
4052+
* Prepared transactions don't hold vxid locks. The
4053+
* LocalTransactionId is always a normal, locked XID.
4054+
*/
4055+
SET_LOCKTAG_TRANSACTION(tag,vxid.localTransactionId);
4056+
lar=LockAcquire(&tag,ShareLock, false, !wait);
4057+
if (lar!=LOCKACQUIRE_NOT_AVAIL)
4058+
LockRelease(&tag,ShareLock, false);
4059+
returnlar!=LOCKACQUIRE_NOT_AVAIL;
4060+
}
4061+
40554062
SET_LOCKTAG_VIRTUALTRANSACTION(tag,vxid);
40564063

40574064
/*

‎src/include/storage/lock.h

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

4343
/*
4444
* Top-level transactions are identified by VirtualTransactionIDs comprising
45-
*the BackendId of the backend running the xact, plus a locally-assigned
46-
* LocalTransactionId. These are guaranteed unique over the short term,
47-
* but will be reused after a database restart; hence they should never
48-
* be stored on disk.
45+
*PGPROC fields backendId and lxid. For prepared transactions, the
46+
* LocalTransactionId is an ordinary XID. These are guaranteed unique over
47+
*the short term,but will be reused after a database restart or XID
48+
*wraparound; hence they should neverbe stored on disk.
4949
*
5050
* Note that struct VirtualTransactionId can not be assumed to be atomically
5151
* assignable as a whole. However, type LocalTransactionId is assumed to
@@ -57,16 +57,16 @@ extern bool Debug_deadlocks;
5757
*/
5858
typedefstruct
5959
{
60-
BackendIdbackendId;/* determined at backend startup */
61-
LocalTransactionIdlocalTransactionId;/* backend-local transaction
62-
* id */
60+
BackendIdbackendId;/* backendId from PGPROC */
61+
LocalTransactionIdlocalTransactionId;/* lxid from PGPROC */
6362
}VirtualTransactionId;
6463

6564
#defineInvalidLocalTransactionId0
6665
#defineLocalTransactionIdIsValid(lxid) ((lxid) != InvalidLocalTransactionId)
6766
#defineVirtualTransactionIdIsValid(vxid) \
68-
(((vxid).backendId != InvalidBackendId) && \
69-
LocalTransactionIdIsValid((vxid).localTransactionId))
67+
(LocalTransactionIdIsValid((vxid).localTransactionId))
68+
#defineVirtualTransactionIdIsPreparedXact(vxid) \
69+
((vxid).backendId == InvalidBackendId)
7070
#defineVirtualTransactionIdEquals(vxid1,vxid2) \
7171
((vxid1).backendId == (vxid2).backendId && \
7272
(vxid1).localTransactionId == (vxid2).localTransactionId)

‎src/test/isolation/Makefile

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,11 @@ installcheck: all
4848
check: all
4949
$(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir=$(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule
5050

51-
# Versions of the check tests that include the prepared_transactions test
52-
# It only makes sense to run these if set up to use prepared transactions,
53-
# via TEMP_CONFIG for the check case, or via the postgresql.conf for the
54-
# installcheck case.
51+
# Non-default tests. It only makes sense to run these if set up to use
52+
# prepared transactions, via TEMP_CONFIG for the check case, or via the
53+
# postgresql.conf for the installcheck case.
5554
installcheck-prepared-txns: all temp-install
56-
./pg_isolation_regress --bindir='$(bindir)'$(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
55+
./pg_isolation_regress --bindir='$(bindir)'$(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic
5756

5857
check-prepared-txns: all temp-install
59-
$(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir=$(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule prepared-transactions
58+
$(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir=$(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic

‎src/test/isolation/README

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ To run just specific test(s), you can do something like
1919
./pg_isolation_regress fk-contention fk-deadlock
2020
(look into the specs/ subdirectory to see the available tests).
2121

22-
The prepared-transactions test requires the server's
23-
max_prepared_transactions parameter to be set to at least 3; therefore it
24-
is not run by default. To include it in the test run, use
22+
Certain tests require the server's max_prepared_transactions parameter to be
23+
set to at least 3; therefore they are not run by default. To include them in
24+
the test run, use
25+
make check-prepared-txns
26+
or
2527
make installcheck-prepared-txns
2628

2729
To define tests with overlapping transactions, we use test specification
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