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

Commiteffe7d9

Browse files
committed
Make release of 2PC identifier and locks consistent in COMMIT PREPARED
When preparing a transaction in two-phase commit, a dummy PGPROC entryholding the GID used for the transaction is registered, which getsreleased once COMMIT PREPARED is run. Prior releasing its shared memorystate, all the locks taken in the prepared transaction are releasedusing a dedicated set of callbacks (pgstat and multixact having similarcallbacks), which may cause the locks to be released before the GID isset free.Hence, there is a small window where lock conflicts could happen, forexample:- Transaction A releases its locks, still holding its GID in sharedmemory.- Transaction B held a lock which conflicted with locks of transactionA.- Transaction B continues its processing, reusing the same GID astransaction A.- Transaction B fails because of a conflicting GID, already in use bytransaction A.This commit changes the shared memory state release so as post-commitcallbacks and predicate lock cleanup happen consistently with the sharedmemory state cleanup for the dummy PGPROC entry. The race window issmall and 2PC had this issue from the start, so no backpatch is done.On top if that fixes discussed involved ABI breakages, which are notwelcome in stable branches.Reported-by: Oleksii Kliukin, Ildar MusinDiagnosed-by: Oleksii Kliukin, Ildar MusinAuthor: Michael PaquierReviewed-by: Masahiko Sawada, Oleksii KliukinDiscussion:https://postgr.es/m/BF9B38A4-2BFF-46E8-BA87-A2D00A8047A6@hintbits.com
1 parent29ddb54 commiteffe7d9

File tree

4 files changed

+44
-19
lines changed

4 files changed

+44
-19
lines changed

‎src/backend/access/transam/multixact.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,7 +1713,7 @@ PostPrepare_MultiXact(TransactionId xid)
17131713
myOldestMember=OldestMemberMXactId[MyBackendId];
17141714
if (MultiXactIdIsValid(myOldestMember))
17151715
{
1716-
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid);
1716+
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid, false);
17171717

17181718
/*
17191719
* Even though storing MultiXactId is atomic, acquire lock to make
@@ -1755,7 +1755,7 @@ void
17551755
multixact_twophase_recover(TransactionIdxid,uint16info,
17561756
void*recdata,uint32len)
17571757
{
1758-
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid);
1758+
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid, false);
17591759
MultiXactIdoldestMember;
17601760

17611761
/*
@@ -1776,7 +1776,7 @@ void
17761776
multixact_twophase_postcommit(TransactionIdxid,uint16info,
17771777
void*recdata,uint32len)
17781778
{
1779-
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid);
1779+
BackendIddummyBackendId=TwoPhaseGetDummyBackendId(xid, true);
17801780

17811781
Assert(len==sizeof(MultiXactId));
17821782

‎src/backend/access/transam/twophase.c

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -801,24 +801,30 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
801801
* TwoPhaseGetGXact
802802
*Get the GlobalTransaction struct for a prepared transaction
803803
*specified by XID
804+
*
805+
* If lock_held is set to true, TwoPhaseStateLock will not be taken, so the
806+
* caller had better hold it.
804807
*/
805808
staticGlobalTransaction
806-
TwoPhaseGetGXact(TransactionIdxid)
809+
TwoPhaseGetGXact(TransactionIdxid,boollock_held)
807810
{
808811
GlobalTransactionresult=NULL;
809812
inti;
810813

811814
staticTransactionIdcached_xid=InvalidTransactionId;
812815
staticGlobalTransactioncached_gxact=NULL;
813816

817+
Assert(!lock_held||LWLockHeldByMe(TwoPhaseStateLock));
818+
814819
/*
815820
* During a recovery, COMMIT PREPARED, or ABORT PREPARED, we'll be called
816821
* repeatedly for the same XID. We can save work with a simple cache.
817822
*/
818823
if (xid==cached_xid)
819824
returncached_gxact;
820825

821-
LWLockAcquire(TwoPhaseStateLock,LW_SHARED);
826+
if (!lock_held)
827+
LWLockAcquire(TwoPhaseStateLock,LW_SHARED);
822828

823829
for (i=0;i<TwoPhaseState->numPrepXacts;i++)
824830
{
@@ -832,7 +838,8 @@ TwoPhaseGetGXact(TransactionId xid)
832838
}
833839
}
834840

835-
LWLockRelease(TwoPhaseStateLock);
841+
if (!lock_held)
842+
LWLockRelease(TwoPhaseStateLock);
836843

837844
if (result==NULL)/* should not happen */
838845
elog(ERROR,"failed to find GlobalTransaction for xid %u",xid);
@@ -849,24 +856,28 @@ TwoPhaseGetGXact(TransactionId xid)
849856
*
850857
* Dummy backend IDs are similar to real backend IDs of real backends.
851858
* They start at MaxBackends + 1, and are unique across all currently active
852-
* real backends and prepared transactions.
859+
* real backends and prepared transactions. If lock_held is set to true,
860+
* TwoPhaseStateLock will not be taken, so the caller had better hold it.
853861
*/
854862
BackendId
855-
TwoPhaseGetDummyBackendId(TransactionIdxid)
863+
TwoPhaseGetDummyBackendId(TransactionIdxid,boollock_held)
856864
{
857-
GlobalTransactiongxact=TwoPhaseGetGXact(xid);
865+
GlobalTransactiongxact=TwoPhaseGetGXact(xid,lock_held);
858866

859867
returngxact->dummyBackendId;
860868
}
861869

862870
/*
863871
* TwoPhaseGetDummyProc
864872
*Get the PGPROC that represents a prepared transaction specified by XID
873+
*
874+
* If lock_held is set to true, TwoPhaseStateLock will not be taken, so the
875+
* caller had better hold it.
865876
*/
866877
PGPROC*
867-
TwoPhaseGetDummyProc(TransactionIdxid)
878+
TwoPhaseGetDummyProc(TransactionIdxid,boollock_held)
868879
{
869-
GlobalTransactiongxact=TwoPhaseGetGXact(xid);
880+
GlobalTransactiongxact=TwoPhaseGetGXact(xid,lock_held);
870881

871882
return&ProcGlobal->allProcs[gxact->pgprocno];
872883
}
@@ -1560,6 +1571,14 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
15601571
if (hdr->initfileinval)
15611572
RelationCacheInitFilePostInvalidate();
15621573

1574+
/*
1575+
* Acquire the two-phase lock. We want to work on the two-phase callbacks
1576+
* while holding it to avoid potential conflicts with other transactions
1577+
* attempting to use the same GID, so the lock is released once the shared
1578+
* memory state is cleared.
1579+
*/
1580+
LWLockAcquire(TwoPhaseStateLock,LW_EXCLUSIVE);
1581+
15631582
/* And now do the callbacks */
15641583
if (isCommit)
15651584
ProcessRecords(bufptr,xid,twophase_postcommit_callbacks);
@@ -1568,6 +1587,15 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
15681587

15691588
PredicateLockTwoPhaseFinish(xid,isCommit);
15701589

1590+
/* Clear shared memory state */
1591+
RemoveGXact(gxact);
1592+
1593+
/*
1594+
* Release the lock as all callbacks are called and shared memory cleanup
1595+
* is done.
1596+
*/
1597+
LWLockRelease(TwoPhaseStateLock);
1598+
15711599
/* Count the prepared xact as committed or aborted */
15721600
AtEOXact_PgStat(isCommit);
15731601

@@ -1577,9 +1605,6 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
15771605
if (gxact->ondisk)
15781606
RemoveTwoPhaseFile(xid, true);
15791607

1580-
LWLockAcquire(TwoPhaseStateLock,LW_EXCLUSIVE);
1581-
RemoveGXact(gxact);
1582-
LWLockRelease(TwoPhaseStateLock);
15831608
MyLockedGxact=NULL;
15841609

15851610
RESUME_INTERRUPTS();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3243,7 +3243,7 @@ AtPrepare_Locks(void)
32433243
void
32443244
PostPrepare_Locks(TransactionIdxid)
32453245
{
3246-
PGPROC*newproc=TwoPhaseGetDummyProc(xid);
3246+
PGPROC*newproc=TwoPhaseGetDummyProc(xid, false);
32473247
HASH_SEQ_STATUSstatus;
32483248
LOCALLOCK*locallock;
32493249
LOCK*lock;
@@ -4034,7 +4034,7 @@ lock_twophase_recover(TransactionId xid, uint16 info,
40344034
void*recdata,uint32len)
40354035
{
40364036
TwoPhaseLockRecord*rec= (TwoPhaseLockRecord*)recdata;
4037-
PGPROC*proc=TwoPhaseGetDummyProc(xid);
4037+
PGPROC*proc=TwoPhaseGetDummyProc(xid, false);
40384038
LOCKTAG*locktag;
40394039
LOCKMODElockmode;
40404040
LOCKMETHODIDlockmethodid;
@@ -4247,7 +4247,7 @@ lock_twophase_postcommit(TransactionId xid, uint16 info,
42474247
void*recdata,uint32len)
42484248
{
42494249
TwoPhaseLockRecord*rec= (TwoPhaseLockRecord*)recdata;
4250-
PGPROC*proc=TwoPhaseGetDummyProc(xid);
4250+
PGPROC*proc=TwoPhaseGetDummyProc(xid, true);
42514251
LOCKTAG*locktag;
42524252
LOCKMETHODIDlockmethodid;
42534253
LockMethodlockMethodTable;

‎src/include/access/twophase.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ extern void TwoPhaseShmemInit(void);
3434
externvoidAtAbort_Twophase(void);
3535
externvoidPostPrepare_Twophase(void);
3636

37-
externPGPROC*TwoPhaseGetDummyProc(TransactionIdxid);
38-
externBackendIdTwoPhaseGetDummyBackendId(TransactionIdxid);
37+
externPGPROC*TwoPhaseGetDummyProc(TransactionIdxid,boollock_held);
38+
externBackendIdTwoPhaseGetDummyBackendId(TransactionIdxid,boollock_held);
3939

4040
externGlobalTransactionMarkAsPreparing(TransactionIdxid,constchar*gid,
4141
TimestampTzprepared_at,

0 commit comments

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