From 4bb7647efef51e53f35bb3a08a361f057ef27707 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 10 Feb 2023 14:29:23 +1300 Subject: [PATCH v6 2/3] fixup! wip-resowner-lock-release-all. --- src/backend/replication/logical/launcher.c | 2 +- src/backend/storage/lmgr/lock.c | 19 +++++-------------- src/backend/utils/resowner/resowner.c | 20 +++++++++++++++++--- src/include/storage/lock.h | 11 +++++------ 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c index 8998b55f62..8ba6a51945 100644 --- a/src/backend/replication/logical/launcher.c +++ b/src/backend/replication/logical/launcher.c @@ -798,7 +798,7 @@ logicalrep_worker_onexit(int code, Datum arg) * parallel apply mode and will not be released when the worker * terminates, so manually release all locks before the worker exits. */ - LockReleaseSession(DEFAULT_LOCKMETHOD); + ProcReleaseLocks(false); ApplyLauncherWakeup(); } diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 7e2dd3a7af..1033d57b88 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -2045,6 +2045,7 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) dlist_delete(&locallockowner->resowner_node); } + /* ensure nLocks didn't go negative */ Assert(locallockowner->nLocks >= 0); } @@ -2066,7 +2067,7 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) if (locallock->nLocks > 0) return true; - Assert(locallock->nLocks >= 0); + Assert(locallock->nLocks == 0); /* * At this point we can no longer suppose we are clear of invalidation @@ -2175,7 +2176,7 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) * LockAssertNoneHeld -- Assert that we no longer hold any DEFAULT_LOCKMETHOD * locks during an abort. */ -extern void +void LockAssertNoneHeld(bool isCommit) { HASH_SEQ_STATUS status; @@ -2238,10 +2239,8 @@ LockReleaseSession(LOCKMETHODID lockmethodid) * Release all locks belonging to 'owner' */ void -LockReleaseCurrentOwner(ResourceOwner owner, dlist_node *resowner_node) +LockReleaseCurrentOwner(ResourceOwner owner, LOCALLOCKOWNER *locallockowner) { - LOCALLOCKOWNER *locallockowner = dlist_container(LOCALLOCKOWNER, resowner_node, resowner_node); - Assert(locallockowner->owner == owner); ReleaseLockIfHeld(locallockowner, false); @@ -2301,18 +2300,10 @@ ReleaseLockIfHeld(LOCALLOCKOWNER *locallockowner, bool sessionLock) * LockReassignCurrentOwner * Reassign all locks belonging to CurrentResourceOwner to belong * to its parent resource owner. - * - * If the caller knows what those locks are, it can pass them as an array. - * That speeds up the call significantly, when a lot of locks are held - * (e.g pg_dump with a large schema). Otherwise, pass NULL for locallocks, - * and we'll traverse through our hash table to find them. */ void -LockReassignCurrentOwner(ResourceOwner owner, dlist_node *resowner_node) +LockReassignCurrentOwner(LOCALLOCKOWNER *locallockowner) { - LOCALLOCKOWNER *locallockowner = dlist_container(LOCALLOCKOWNER, - resowner_node, - resowner_node); ResourceOwner parent = ResourceOwnerGetParent(CurrentResourceOwner); LockReassignOwner(locallockowner, parent); diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index 321ea15c78..46a9a3ca42 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -573,7 +573,11 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, if (isTopLevel) { dlist_foreach_modify(iter, &owner->locks) - LockReleaseCurrentOwner(owner, iter.cur); + { + LOCALLOCKOWNER *locallockowner = dlist_container(LOCALLOCKOWNER, resowner_node, iter.cur); + + LockReleaseCurrentOwner(owner, locallockowner); + } Assert(dlist_is_empty(&owner->locks)); @@ -598,14 +602,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, if (isCommit) { dlist_foreach_modify(iter, &owner->locks) - LockReassignCurrentOwner(owner, iter.cur); + { + LOCALLOCKOWNER *locallockowner = dlist_container(LOCALLOCKOWNER, + resowner_node, + iter.cur); + + LockReassignCurrentOwner(locallockowner); + } Assert(dlist_is_empty(&owner->locks)); } else { dlist_foreach_modify(iter, &owner->locks) - LockReleaseCurrentOwner(owner, iter.cur); + { + LOCALLOCKOWNER *locallockowner = dlist_container(LOCALLOCKOWNER, resowner_node, iter.cur); + + LockReleaseCurrentOwner(owner, locallockowner); + } Assert(dlist_is_empty(&owner->locks)); } diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index f2617f805e..e3861a8ea5 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -417,11 +417,11 @@ typedef struct LOCALLOCKOWNER */ struct ResourceOwnerData *owner; - dlist_node resowner_node; + dlist_node resowner_node; /* dlist link for ResourceOwner.locks */ - dlist_node locallock_node; + dlist_node locallock_node; /* dlist link for LOCALLOCK.locallockowners */ - struct LOCALLOCK *locallock; + struct LOCALLOCK *locallock; /* pointer to the corresponding LOCALLOCK */ int64 nLocks; /* # of times held by this owner */ } LOCALLOCKOWNER; @@ -575,9 +575,8 @@ extern void LockAssertNoneHeld(bool isCommit); extern void LockReleaseSession(LOCKMETHODID lockmethodid); struct ResourceOwnerData; extern void LockReleaseCurrentOwner(struct ResourceOwnerData *owner, - dlist_node *resowner_node); -extern void LockReassignCurrentOwner(struct ResourceOwnerData *owner, - dlist_node *resowner_node); + LOCALLOCKOWNER *locallockowner); +extern void LockReassignCurrentOwner(LOCALLOCKOWNER *locallockowner); extern bool LockHeldByMe(const LOCKTAG *locktag, LOCKMODE lockmode); #ifdef USE_ASSERT_CHECKING extern HTAB *GetLockMethodLocalHash(void); -- 2.37.2