From 3a8c57e622bff66b5b0f33a3356e5d69c7b7c1e8 Mon Sep 17 00:00:00 2001 From: Will Mortensen Date: Thu, 21 Dec 2023 22:08:51 -0800 Subject: [PATCH v7 1/3] Refactor GetLockConflicts() into more general GetLockers() This also supports getting lockers in a single specified lock mode, rather than all modes that conflict with a specified mode. --- 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 | 4 +- src/backend/storage/lmgr/lock.c | 67 ++++++++++++++++----------- src/backend/storage/lmgr/proc.c | 4 +- src/include/storage/lock.h | 5 +- 7 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 8426458f7f..41394c88fa 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -473,7 +473,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->lxid = xid; proc->backendId = InvalidBackendId; } diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 464858117e..7d4be438c2 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2584,8 +2584,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 d8755a106d..d59cb2d348 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -657,7 +657,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict) */ VirtualTransactionId *backends; - backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL); + backends = GetLockers(&locktag, AccessExclusiveLock, true, NULL); /* * Prevent ResolveRecoveryConflictWithVirtualXIDs() from reporting @@ -711,7 +711,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 4975d4b67d..a0e3eea3a0 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -923,8 +923,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; } diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index c70a1adb9a..0542c83f1f 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -2833,43 +2833,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); /* @@ -2890,19 +2897,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; @@ -2955,12 +2970,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)) @@ -2975,7 +2990,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; /* @@ -3009,11 +3024,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; @@ -3022,8 +3037,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)) @@ -3039,7 +3052,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].backendId = InvalidBackendId; vxids[count].localTransactionId = InvalidLocalTransactionId; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index e5977548fe..6490fbb7ee 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1299,8 +1299,8 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) 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 00679624f7..bf8ca45ff8 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -574,8 +574,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