From 649f3bc8adfe2ba4ad8723740d457c445ef7cb9a Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Sat, 14 Mar 2020 12:43:35 +0530 Subject: [PATCH v10 1/4] Assert that we don't acquire a heavyweight lock on another object after relation extension lock. The only exception to the rule is that we can try to acquire the same relation extension lock more than once. This is allowed as we are not creating any new lock for this case. This restriction implies that the relation extension lock won't ever participate in the deadlock cycle because we can never wait for any other heavyweight lock after acquiring this lock. Such a restriction is okay for relation extension locks as unlike other heavyweight locks these are not held till the transaction end. These are taken for a short duration to extend a particular relation and then released. Author: Dilip Kumar, with few changes by Amit Kapila Reviewed-by: Amit Kapila and Kuntal Ghosh Discussion: https://postgr.es/m/CAD21AoCmT3cFQUN4aVvzy5chw7DuzXrJCbrjTU05B+Ss=Gn1LA@mail.gmail.com --- src/backend/storage/lmgr/lock.c | 53 +++++++++++++++++++++++++++++++++++++++++ src/include/storage/lock.h | 1 + 2 files changed, 54 insertions(+) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 1df7b8e..34a7ed9 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -170,6 +170,23 @@ typedef struct TwoPhaseLockRecord */ static int FastPathLocalUseCount = 0; +/* + * Flag to indicate if the relation extension lock is held by this backend. + * This flag is used to ensure that while holding the relation extension lock + * we don't try to acquire a heavyweight lock on any other object. This + * restriction implies that the relation extension lock won't ever participate + * in the deadlock cycle because we can never wait for any other heavyweight + * lock after acquiring this lock. + * + * Such a restriction is okay for relation extension locks as unlike other + * heavyweight locks these are not held till the transaction end. These are + * taken for a short duration to extend a particular relation and then + * released. + */ +#ifdef USE_ASSERT_CHECKING +static bool IsRelationExtensionLockHeld = false; +#endif + /* Macros for manipulating proc->fpLockBits */ #define FAST_PATH_BITS_PER_SLOT 3 #define FAST_PATH_LOCKNUMBER_OFFSET 1 @@ -841,6 +858,13 @@ LockAcquireExtended(const LOCKTAG *locktag, } /* + * We don't acquire any other heavyweight lock while holding the relation + * extension lock. We do allow to acquire the same relation extension + * lock more than once but that case won't reach here. + */ + Assert(!IsRelationExtensionLockHeld); + + /* * Prepare to emit a WAL record if acquisition of this lock needs to be * replayed in a standby server. * @@ -1288,6 +1312,23 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc, } /* + * Check and set/reset the flag that we hold the relation extension lock. + * + * It is callers responsibility that this function is called after + * acquiring/releasing the relation extension lock. + * + * Pass acquired = true if lock is acquired, false otherwise. + */ +#ifdef USE_ASSERT_CHECKING +static inline void +CheckAndSetLockHeld(LOCALLOCK *locallock, bool acquired) +{ + if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_RELATION_EXTEND) + IsRelationExtensionLockHeld = acquired; +} +#endif + +/* * Subroutine to free a locallock entry */ static void @@ -1322,6 +1363,13 @@ RemoveLocalLock(LOCALLOCK *locallock) (void *) &(locallock->tag), HASH_REMOVE, NULL)) elog(WARNING, "locallock table corrupted"); + + /* + * Indicate that the lock is released for a particular type of locks. + */ +#ifdef USE_ASSERT_CHECKING + CheckAndSetLockHeld(locallock, false); +#endif } /* @@ -1618,6 +1666,11 @@ GrantLockLocal(LOCALLOCK *locallock, ResourceOwner owner) locallock->numLockOwners++; if (owner != NULL) ResourceOwnerRememberLock(owner, locallock); + + /* Indicate that the lock is acquired for a certain type of locks. */ +#ifdef USE_ASSERT_CHECKING + CheckAndSetLockHeld(locallock, true); +#endif } /* diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index bb8e4e6..fc0a712 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -419,6 +419,7 @@ typedef struct LOCALLOCK } LOCALLOCK; #define LOCALLOCK_LOCKMETHOD(llock) ((llock).tag.lock.locktag_lockmethodid) +#define LOCALLOCK_LOCKTAG(llock) ((LockTagType) (llock).tag.lock.locktag_type) /* -- 1.8.3.1