From 2e5005cf2a1fb4f6d21b786327e733a5722c0d77 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Sat, 14 Mar 2020 17:09:23 +0530 Subject: [PATCH 2/4] Add assert to ensure that page locks don't participate in deadlock cycle. Assert that we don't acquire any other heavyweight lock while holding the page lock except for relation extension. However, these locks are never taken in reverse order which implies that page locks will never participate in the deadlock cycle. Similar to relation extension, page locks are also held for a short duration, so imposing such a restriction won't hurt. 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 | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 2ff7c31..f0c913f0 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -185,6 +185,18 @@ static int FastPathLocalUseCount = 0; */ static bool IsRelationExtensionLockHeld = false; +/* + * Flag to indicate if the page lock is held by this backend. We don't + * acquire any other heavyweight lock while holding the page lock except for + * relation extension. However, these locks are never taken in reverse order + * which implies that page locks will also never participate in the deadlock + * cycle. + * + * Similar to relation extension, page locks are also held for a short + * duration, so imposing such a restriction won't hurt. + */ +static bool IsPageLockHeld = false; + /* Macros for manipulating proc->fpLockBits */ #define FAST_PATH_BITS_PER_SLOT 3 #define FAST_PATH_LOCKNUMBER_OFFSET 1 @@ -863,6 +875,13 @@ LockAcquireExtended(const LOCKTAG *locktag, Assert(!IsRelationExtensionLockHeld); /* + * We don't acquire any other heavyweight lock while holding the page lock + * except for relation extension lock. + */ + Assert(!IsPageLockHeld || + (locktag->locktag_type == LOCKTAG_RELATION_EXTEND)); + + /* * Prepare to emit a WAL record if acquisition of this lock needs to be * replayed in a standby server. * @@ -1310,30 +1329,34 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc, } /* - * Check and set the flag that we hold the relation extension lock. + * Check and set the flag that we hold the relation extension/page lock. * * It is callers responsibility that this function is called after acquiring - * the relation extension lock. + * the relation extension/page lock. */ static inline void CheckAndSetLockHeld(LOCALLOCK *locallock) { if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_RELATION_EXTEND) IsRelationExtensionLockHeld = true; + else if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_PAGE) + IsPageLockHeld = true; } /* * Check and reset the flag to indicate that we have released the relation - * extension lock. + * extension/page lock. * * It is callers responsibility to ensure that this function is called after - * releasing the relation extension lock. + * releasing the relation extension/page lock. */ static inline void CheckAndResetLockHeld(LOCALLOCK *locallock) { if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_RELATION_EXTEND) IsRelationExtensionLockHeld = false; + else if (LOCALLOCK_LOCKTAG(*locallock) == LOCKTAG_PAGE) + IsPageLockHeld = false; } /* -- 1.8.3.1