From a5e4594ecc1f363798fea4691c74c5559d56777c Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Sat, 14 Mar 2020 12:43:35 +0530 Subject: [PATCH 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 | 57 +++++++++++++++++++++++++++++++++++++++++ src/include/storage/lock.h | 1 + 2 files changed, 58 insertions(+) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 1df7b8e..2ff7c31 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -170,6 +170,21 @@ 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. + */ +static bool IsRelationExtensionLockHeld = false; + /* Macros for manipulating proc->fpLockBits */ #define FAST_PATH_BITS_PER_SLOT 3 #define FAST_PATH_LOCKNUMBER_OFFSET 1 @@ -841,6 +856,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 +1310,33 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc, } /* + * Check and set the flag that we hold the relation extension lock. + * + * It is callers responsibility that this function is called after acquiring + * the relation extension lock. + */ +static inline void +CheckAndSetLockHeld(LOCALLOCK *locallock) +{ + if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_RELATION_EXTEND) + IsRelationExtensionLockHeld = true; +} + +/* + * Check and reset the flag to indicate that we have released the relation + * extension lock. + * + * It is callers responsibility to ensure that this function is called after + * releasing the relation extension lock. + */ +static inline void +CheckAndResetLockHeld(LOCALLOCK *locallock) +{ + if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_RELATION_EXTEND) + IsRelationExtensionLockHeld = false; +} + +/* * Subroutine to free a locallock entry */ static void @@ -1322,6 +1371,11 @@ 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. + */ + CheckAndResetLockHeld(locallock); } /* @@ -1618,6 +1672,9 @@ 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. */ + CheckAndSetLockHeld(locallock); } /* 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