diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 07ae7a31db5..c3998719405 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -1713,7 +1713,7 @@ PostPrepare_MultiXact(TransactionId xid) myOldestMember = OldestMemberMXactId[MyBackendId]; if (MultiXactIdIsValid(myOldestMember)) { - BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid); + BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid, false); /* * Even though storing MultiXactId is atomic, acquire lock to make @@ -1755,7 +1755,7 @@ void multixact_twophase_recover(TransactionId xid, uint16 info, void *recdata, uint32 len) { - BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid); + BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid, false); MultiXactId oldestMember; /* @@ -1776,7 +1776,7 @@ void multixact_twophase_postcommit(TransactionId xid, uint16 info, void *recdata, uint32 len) { - BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid); + BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid, true); Assert(len == sizeof(MultiXactId)); diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index add979bf0b2..64679dd2de9 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -801,9 +801,12 @@ pg_prepared_xact(PG_FUNCTION_ARGS) * TwoPhaseGetGXact * Get the GlobalTransaction struct for a prepared transaction * specified by XID + * + * If lock_held is set to true, TwoPhaseStateLock will not be taken, so the + * caller had better hold it. */ static GlobalTransaction -TwoPhaseGetGXact(TransactionId xid) +TwoPhaseGetGXact(TransactionId xid, bool lock_held) { GlobalTransaction result = NULL; int i; @@ -811,6 +814,8 @@ TwoPhaseGetGXact(TransactionId xid) static TransactionId cached_xid = InvalidTransactionId; static GlobalTransaction cached_gxact = NULL; + Assert(!lock_held || LWLockHeldByMe(TwoPhaseStateLock)); + /* * During a recovery, COMMIT PREPARED, or ABORT PREPARED, we'll be called * repeatedly for the same XID. We can save work with a simple cache. @@ -818,7 +823,8 @@ TwoPhaseGetGXact(TransactionId xid) if (xid == cached_xid) return cached_gxact; - LWLockAcquire(TwoPhaseStateLock, LW_SHARED); + if (!lock_held) + LWLockAcquire(TwoPhaseStateLock, LW_SHARED); for (i = 0; i < TwoPhaseState->numPrepXacts; i++) { @@ -832,7 +838,8 @@ TwoPhaseGetGXact(TransactionId xid) } } - LWLockRelease(TwoPhaseStateLock); + if (!lock_held) + LWLockRelease(TwoPhaseStateLock); if (result == NULL) /* should not happen */ elog(ERROR, "failed to find GlobalTransaction for xid %u", xid); @@ -849,12 +856,13 @@ TwoPhaseGetGXact(TransactionId xid) * * Dummy backend IDs are similar to real backend IDs of real backends. * They start at MaxBackends + 1, and are unique across all currently active - * real backends and prepared transactions. + * real backends and prepared transactions. If lock_held is set to true, + * TwoPhaseStateLock will not be taken, so the caller had better hold it. */ BackendId -TwoPhaseGetDummyBackendId(TransactionId xid) +TwoPhaseGetDummyBackendId(TransactionId xid, bool lock_held) { - GlobalTransaction gxact = TwoPhaseGetGXact(xid); + GlobalTransaction gxact = TwoPhaseGetGXact(xid, lock_held); return gxact->dummyBackendId; } @@ -862,11 +870,14 @@ TwoPhaseGetDummyBackendId(TransactionId xid) /* * TwoPhaseGetDummyProc * Get the PGPROC that represents a prepared transaction specified by XID + * + * If lock_held is set to true, TwoPhaseStateLock will not be taken, so the + * caller had better hold it. */ PGPROC * -TwoPhaseGetDummyProc(TransactionId xid) +TwoPhaseGetDummyProc(TransactionId xid, bool lock_held) { - GlobalTransaction gxact = TwoPhaseGetGXact(xid); + GlobalTransaction gxact = TwoPhaseGetGXact(xid, lock_held); return &ProcGlobal->allProcs[gxact->pgprocno]; } @@ -1560,6 +1571,14 @@ FinishPreparedTransaction(const char *gid, bool isCommit) if (hdr->initfileinval) RelationCacheInitFilePostInvalidate(); + /* + * Acquire the two-phase lock. We want to work on the two-phase callbacks + * while holding it to avoid potential conflicts with other transactions + * attempting to use the same GID, so the lock is released once the shared + * memory state is cleared. + */ + LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); + /* And now do the callbacks */ if (isCommit) ProcessRecords(bufptr, xid, twophase_postcommit_callbacks); @@ -1568,6 +1587,15 @@ FinishPreparedTransaction(const char *gid, bool isCommit) PredicateLockTwoPhaseFinish(xid, isCommit); + /* Clear shared memory state */ + RemoveGXact(gxact); + + /* + * Release the lock as all callbacks are called and shared memory cleanup + * is done. + */ + LWLockRelease(TwoPhaseStateLock); + /* Count the prepared xact as committed or aborted */ AtEOXact_PgStat(isCommit); @@ -1577,9 +1605,6 @@ FinishPreparedTransaction(const char *gid, bool isCommit) if (gxact->ondisk) RemoveTwoPhaseFile(xid, true); - LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); - RemoveGXact(gxact); - LWLockRelease(TwoPhaseStateLock); MyLockedGxact = NULL; RESUME_INTERRUPTS(); diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 3bb5ce350aa..78fdbd6ff88 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -3243,7 +3243,7 @@ AtPrepare_Locks(void) void PostPrepare_Locks(TransactionId xid) { - PGPROC *newproc = TwoPhaseGetDummyProc(xid); + PGPROC *newproc = TwoPhaseGetDummyProc(xid, false); HASH_SEQ_STATUS status; LOCALLOCK *locallock; LOCK *lock; @@ -4034,7 +4034,7 @@ lock_twophase_recover(TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhaseLockRecord *rec = (TwoPhaseLockRecord *) recdata; - PGPROC *proc = TwoPhaseGetDummyProc(xid); + PGPROC *proc = TwoPhaseGetDummyProc(xid, false); LOCKTAG *locktag; LOCKMODE lockmode; LOCKMETHODID lockmethodid; @@ -4247,7 +4247,7 @@ lock_twophase_postcommit(TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhaseLockRecord *rec = (TwoPhaseLockRecord *) recdata; - PGPROC *proc = TwoPhaseGetDummyProc(xid); + PGPROC *proc = TwoPhaseGetDummyProc(xid, true); LOCKTAG *locktag; LOCKMETHODID lockmethodid; LockMethod lockMethodTable; diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h index 6228b091d89..fcd1913c43a 100644 --- a/src/include/access/twophase.h +++ b/src/include/access/twophase.h @@ -34,8 +34,8 @@ extern void TwoPhaseShmemInit(void); extern void AtAbort_Twophase(void); extern void PostPrepare_Twophase(void); -extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid); -extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid); +extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid, bool lock_held); +extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid, bool lock_held); extern GlobalTransaction MarkAsPreparing(TransactionId xid, const char *gid, TimestampTz prepared_at,