From e2b49b591b3d61849f37fd69cef518c617254451 Mon Sep 17 00:00:00 2001 From: Will Mortensen Date: Thu, 21 Dec 2023 22:12:49 -0800 Subject: [PATCH v8 2/3] Allow specifying single lockmode in WaitForLockers() Allow waiting for a single specified lock mode, rather than all lock modes that conflict with a specified mode. --- src/backend/catalog/index.c | 4 ++-- src/backend/commands/indexcmds.c | 12 ++++++------ src/backend/commands/tablecmds.c | 3 ++- src/backend/storage/lmgr/lmgr.c | 25 +++++++++++++------------ src/include/storage/lmgr.h | 6 ++++-- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 4b88a9cb87..09c64997c4 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2303,7 +2303,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) * here, even though it will only be used when we're called by REINDEX * CONCURRENTLY and not when called by DROP INDEX CONCURRENTLY. */ - WaitForLockers(heaplocktag, AccessExclusiveLock, true); + WaitForLockers(heaplocktag, AccessExclusiveLock, true, true); /* Finish invalidation of index and mark it as dead */ index_concurrently_set_dead(heapId, indexId); @@ -2319,7 +2319,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) * Wait till every transaction that saw the old index state has * finished. See above about progress reporting. */ - WaitForLockers(heaplocktag, AccessExclusiveLock, true); + WaitForLockers(heaplocktag, AccessExclusiveLock, true, true); /* * Re-open relations to allow us to complete our actions. diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 7a87626f5f..c43625375b 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1674,7 +1674,7 @@ DefineIndex(Oid tableId, * exclusive lock on our table. The lock code will detect deadlock and * error out properly. */ - WaitForLockers(heaplocktag, ShareLock, true); + WaitForLockers(heaplocktag, ShareLock, true, true); /* * At this moment we are sure that there are no transactions with the @@ -1721,7 +1721,7 @@ DefineIndex(Oid tableId, */ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_WAIT_2); - WaitForLockers(heaplocktag, ShareLock, true); + WaitForLockers(heaplocktag, ShareLock, true, true); /* * Now take the "reference snapshot" that will be used by validate_index() @@ -4039,7 +4039,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_WAIT_1); - WaitForLockersMultiple(lockTags, ShareLock, true); + WaitForLockersMultiple(lockTags, ShareLock, true, true); CommitTransactionCommand(); foreach(lc, newIndexIds) @@ -4098,7 +4098,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_WAIT_2); - WaitForLockersMultiple(lockTags, ShareLock, true); + WaitForLockersMultiple(lockTags, ShareLock, true, true); CommitTransactionCommand(); foreach(lc, newIndexIds) @@ -4256,7 +4256,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_WAIT_4); - WaitForLockersMultiple(lockTags, AccessExclusiveLock, true); + WaitForLockersMultiple(lockTags, AccessExclusiveLock, true, true); foreach(lc, indexIds) { @@ -4290,7 +4290,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_WAIT_5); - WaitForLockersMultiple(lockTags, AccessExclusiveLock, true); + WaitForLockersMultiple(lockTags, AccessExclusiveLock, true, true); PushActiveSnapshot(GetTransactionSnapshot()); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 68f658e834..72007e13b4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19615,7 +19615,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * partition itself, since we will acquire AccessExclusiveLock below. */ SET_LOCKTAG_RELATION(tag, MyDatabaseId, parentrelid); - WaitForLockersMultiple(list_make1(&tag), AccessExclusiveLock, false); + WaitForLockersMultiple(list_make1(&tag), AccessExclusiveLock, true, + false); /* * Now acquire locks in both relations again. Note they may have been diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index a0e3eea3a0..885291bba1 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -893,19 +893,20 @@ XactLockTableWaitErrorCb(void *arg) /* * WaitForLockersMultiple - * Wait until no transaction holds locks that conflict with the given - * locktags at the given lockmode. + * Wait until no transaction holds locks on the given locktags, either in + * or conflicting with the given lockmode, depending on the value of the + * conflicting argument. * * To do this, obtain the current list of lockers, and wait on their VXIDs * until they are finished. * * Note we don't try to acquire the locks on the given locktags, only the - * VXIDs and XIDs of their lock holders; if somebody grabs a conflicting lock - * on the objects after we obtained our initial list of lockers, we will not - * wait for them. + * VXIDs and XIDs of their lock holders; if somebody grabs a lock on the objects + * after we obtained our initial list of lockers, we will not wait for them. */ void -WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) +WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool conflicting, + bool progress) { List *holders = NIL; ListCell *lc; @@ -923,7 +924,7 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) int count; holders = lappend(holders, - GetLockers(locktag, lockmode, true, + GetLockers(locktag, lockmode, conflicting, progress ? &count : NULL)); if (progress) total += count; @@ -933,8 +934,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 */ @@ -983,16 +984,16 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) * Same as WaitForLockersMultiple, for a single lock tag. */ void -WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress) +WaitForLockers(LOCKTAG heaplocktag, LOCKMASK lockmask, bool conflicting, + bool progress) { List *l; l = list_make1(&heaplocktag); - WaitForLockersMultiple(l, lockmode, progress); + WaitForLockersMultiple(l, lockmask, conflicting, progress); list_free(l); } - /* * LockDatabaseObject * diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h index e8bd71ba68..7468a0500f 100644 --- a/src/include/storage/lmgr.h +++ b/src/include/storage/lmgr.h @@ -82,8 +82,10 @@ extern void XactLockTableWait(TransactionId xid, Relation rel, extern bool ConditionalXactLockTableWait(TransactionId xid); /* Lock VXIDs, specified by conflicting locktags */ -extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress); -extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress); +extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, + bool conflicting ,bool progress); +extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, + bool conflicting, bool progress); /* Lock an XID for tuple insertion (used to wait for an insertion to finish) */ extern uint32 SpeculativeInsertionLockAcquire(TransactionId xid); -- 2.34.1