From 08799a70408b55fd2ae9101c3481b283088c2186 Mon Sep 17 00:00:00 2001 From: Will Mortensen Date: Thu, 21 Dec 2023 22:08:51 -0800 Subject: [PATCH v12 1/3] Refactor GetLockConflicts() into more general GetLockers() Support getting lockers in a single specified lock mode, rather than all modes that conflict with a specified mode. This allows getting transactions that hold RowExclusiveLock without also getting transactions that hold ShareUpdateExclusiveLock (held by vacuum/autovacuum), among other modes. --- src/backend/access/transam/twophase.c | 2 +- src/backend/access/transam/xact.c | 4 +- src/backend/storage/ipc/standby.c | 4 +- src/backend/storage/lmgr/lmgr.c | 8 ++-- src/backend/storage/lmgr/lock.c | 67 ++++++++++++++++----------- src/backend/storage/lmgr/proc.c | 4 +- src/include/storage/lock.h | 5 +- 7 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index bf451d42ff..44d5da8730 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -454,7 +454,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, else { Assert(AmStartupProcess() || !IsPostmasterEnvironment); - /* GetLockConflicts() uses this to specify a wait on the XID */ + /* GetLockers() uses this to specify a wait on the XID */ proc->vxid.lxid = xid; proc->vxid.procNumber = INVALID_PROC_NUMBER; } diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 4f4ce75762..baba0765dd 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2636,8 +2636,8 @@ PrepareTransaction(void) /* * Transfer our locks to a dummy PGPROC. This has to be done before - * ProcArrayClearTransaction(). Otherwise, a GetLockConflicts() would - * conclude "xact already committed or aborted" for our locks. + * ProcArrayClearTransaction(). Otherwise, a GetLockers() would conclude + * "xact already committed or aborted" for our locks. */ PostPrepare_Locks(xid); diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index 87b04e51b3..17f0762edd 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -656,7 +656,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict) */ VirtualTransactionId *backends; - backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL); + backends = GetLockers(&locktag, AccessExclusiveLock, true, NULL); /* * Prevent ResolveRecoveryConflictWithVirtualXIDs() from reporting @@ -710,7 +710,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict) { VirtualTransactionId *backends; - backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL); + backends = GetLockers(&locktag, AccessExclusiveLock, true, NULL); /* Quick exit if there's no work to be done */ if (!VirtualTransactionIdIsValid(*backends)) diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index fe3cda2f88..720fb1c6a9 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -922,8 +922,8 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) int count; holders = lappend(holders, - GetLockConflicts(locktag, lockmode, - progress ? &count : NULL)); + GetLockers(locktag, lockmode, true, + progress ? &count : NULL)); if (progress) total += count; } @@ -932,8 +932,8 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, total); /* - * Note: GetLockConflicts() never reports our own xid, hence we need not - * check for that. Also, prepared xacts are reported and awaited. + * Note: GetLockers() never reports our own xid, hence we need not check for + * that. Also, prepared xacts are reported and awaited. */ /* Finally wait for each such transaction to complete */ diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index f68c595c8a..eed642acb7 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -2850,43 +2850,50 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock) } /* - * GetLockConflicts + * GetLockers * Get an array of VirtualTransactionIds of xacts currently holding locks - * that would conflict with the specified lock/lockmode. - * xacts merely awaiting such a lock are NOT reported. + * on the specified locktag either in or conflicting with the given + * lockmode, depending on the value of the conflicting argument. xacts + * merely awaiting such a lock are NOT reported. * * The result array is palloc'd and is terminated with an invalid VXID. * *countp, if not null, is updated to the number of items set. * * Of course, the result could be out of date by the time it's returned, so * use of this function has to be thought about carefully. Similarly, a - * PGPROC with no "lxid" will be considered non-conflicting regardless of any - * lock it holds. Existing callers don't care about a locker after that - * locker's pg_xact updates complete. CommitTransaction() clears "lxid" after - * pg_xact updates and before releasing locks. + * PGPROC with no "lxid" will not be returned regardless of any lock it holds. + * Existing callers don't care about a locker after that locker's pg_xact + * updates complete. CommitTransaction() clears "lxid" after pg_xact updates + * and before releasing locks. * - * Note we never include the current xact's vxid in the result array, - * since an xact never blocks itself. + * Note we never include the current xact's vxid in the result array, because + * existing callers don't care to know about it, since an xact never blocks + * itself and can see its own uncommitted changes. */ VirtualTransactionId * -GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) +GetLockers(const LOCKTAG *locktag, LOCKMODE lockmode, bool conflicting, + int *countp) { static VirtualTransactionId *vxids; LOCKMETHODID lockmethodid = locktag->locktag_lockmethodid; LockMethod lockMethodTable; + int numLockModes; LOCK *lock; - LOCKMASK conflictMask; + LOCKMASK getMask; dlist_iter proclock_iter; PROCLOCK *proclock; uint32 hashcode; LWLock *partitionLock; int count = 0; + int i; + bool checkFast = false; int fast_count = 0; if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods)) elog(ERROR, "unrecognized lock method: %d", lockmethodid); lockMethodTable = LockMethods[lockmethodid]; - if (lockmode <= 0 || lockmode > lockMethodTable->numLockModes) + numLockModes = lockMethodTable->numLockModes; + if (lockmode <= 0 || lockmode > numLockModes) elog(ERROR, "unrecognized lock mode: %d", lockmode); /* @@ -2907,19 +2914,27 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) palloc0(sizeof(VirtualTransactionId) * (MaxBackends + max_prepared_xacts + 1)); - /* Compute hash code and partition lock, and look up conflicting modes. */ + /* Compute hash code and partition lock, and construct lock mask */ hashcode = LockTagHashCode(locktag); partitionLock = LockHashPartitionLock(hashcode); - conflictMask = lockMethodTable->conflictTab[lockmode]; + getMask = conflicting ? lockMethodTable->conflictTab[lockmode] : + LOCKBIT_ON(lockmode); /* * Fast path locks might not have been entered in the primary lock table. - * If the lock we're dealing with could conflict with such a lock, we must - * examine each backend's fast-path array for conflicts. + * If getMask could match such a lock, we must examine each backend's + * fast-path array. */ - if (ConflictsWithRelationFastPath(locktag, lockmode)) + for (i = 1; i <= numLockModes; i++) + { + if (((getMask & LOCKBIT_ON(i)) != 0) && + EligibleForRelationFastPath(locktag, i)) { + checkFast = true; + break; + } + } + if (checkFast) { - int i; Oid relid = locktag->locktag_field2; VirtualTransactionId vxid; @@ -2972,12 +2987,12 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) /* * There can only be one entry per relation, so if we found it - * and it doesn't conflict, we can skip the rest of the slots. + * and it doesn't match, we can skip the rest of the slots. */ - if ((lockmask & conflictMask) == 0) + if ((lockmask & getMask) == 0) break; - /* Conflict! */ + /* Match! */ GET_VXID_FROM_PGPROC(vxid, *proc); if (VirtualTransactionIdIsValid(vxid)) @@ -2992,7 +3007,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) } } - /* Remember how many fast-path conflicts we found. */ + /* Remember how many fast-path matches we found. */ fast_count = count; /* @@ -3026,11 +3041,11 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) { proclock = dlist_container(PROCLOCK, lockLink, proclock_iter.cur); - if (conflictMask & proclock->holdMask) + if (getMask & proclock->holdMask) { PGPROC *proc = proclock->tag.myProc; - /* A backend never blocks itself */ + /* A backend doesn't care about its own locks */ if (proc != MyProc) { VirtualTransactionId vxid; @@ -3039,8 +3054,6 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) if (VirtualTransactionIdIsValid(vxid)) { - int i; - /* Avoid duplicate entries. */ for (i = 0; i < fast_count; ++i) if (VirtualTransactionIdEquals(vxids[i], vxid)) @@ -3056,7 +3069,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp) LWLockRelease(partitionLock); if (count > MaxBackends + max_prepared_xacts) /* should never happen */ - elog(PANIC, "too many conflicting locks found"); + elog(PANIC, "too many locks found"); vxids[count].procNumber = INVALID_PROC_NUMBER; vxids[count].localTransactionId = InvalidLocalTransactionId; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index ce29da9012..4964cb9417 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1333,8 +1333,8 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable, bool dontWait) VirtualTransactionId *vxids; int cnt; - vxids = GetLockConflicts(&locallock->tag.lock, - AccessExclusiveLock, &cnt); + vxids = GetLockers(&locallock->tag.lock, + AccessExclusiveLock, true, &cnt); /* * Log the recovery conflict and the list of PIDs of diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 0017d4b868..de40dfaa16 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -573,8 +573,9 @@ extern HTAB *GetLockMethodLocalHash(void); #endif extern bool LockHasWaiters(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock); -extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag, - LOCKMODE lockmode, int *countp); +extern VirtualTransactionId *GetLockers(const LOCKTAG *locktag, + LOCKMODE lockmode, bool conflicting, + int *countp); extern void AtPrepare_Locks(void); extern void PostPrepare_Locks(TransactionId xid); extern bool LockCheckConflicts(LockMethod lockMethodTable, -- 2.34.1