v7-0001-Assert-that-we-don-t-acquire-a-heavyweight-lock-on-a.patch

application/octet-stream

Filename: v7-0001-Assert-that-we-don-t-acquire-a-heavyweight-lock-on-a.patch
Type: application/octet-stream
Part: 0
Message: Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v7-0001
Subject: Assert that we don't acquire a heavyweight lock on another object after relation extension lock.
File+
src/backend/storage/lmgr/lock.c 57 0
src/include/storage/lock.h 1 0
From a5e4594ecc1f363798fea4691c74c5559d56777c Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
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