From 92e9a07d3d6b1e11cab83b3a49a705665ae22e6a Mon Sep 17 00:00:00 2001 From: Will Mortensen Date: Thu, 21 Dec 2023 22:12:49 -0800 Subject: [PATCH v11 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 | 21 +++++++++++---------- src/include/storage/lmgr.h | 6 ++++-- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b6a7c60e23..72e4c92c75 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2272,7 +2272,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); @@ -2288,7 +2288,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 d9016ef487..81711e9afc 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1678,7 +1678,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 @@ -1725,7 +1725,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() @@ -4054,7 +4054,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) @@ -4113,7 +4113,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) @@ -4271,7 +4271,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) { @@ -4305,7 +4305,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 8a02c5b05b..1e4a69c2b7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19837,7 +19837,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 53d838e971..5f552f4d7c 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -892,19 +892,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; @@ -922,7 +923,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; @@ -982,16 +983,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, LOCKMODE lockmode, bool conflicting, + bool progress) { List *l; l = list_make1(&heaplocktag); - WaitForLockersMultiple(l, lockmode, progress); + WaitForLockersMultiple(l, lockmode, conflicting, progress); list_free(l); } - /* * LockDatabaseObject * diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h index e8bd71ba68..2f88b9b6e3 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