From d91d4aafcc4cac4344a0674e3119c6c504884810 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 23 Nov 2022 11:22:56 +0000
Subject: [PATCH v1] Decouple last important WAL record LSN from WAL insert
 locks

---
 src/backend/access/transam/xlog.c | 57 +++++++++++++------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a31fbbff78..882e968c8f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -363,21 +363,11 @@ typedef struct XLogwrtResult
  * the WAL record is just copied to the page and the lock is released. But
  * to avoid the deadlock-scenario explained above, the indicator is always
  * updated before sleeping while holding an insertion lock.
- *
- * lastImportantAt contains the LSN of the last important WAL record inserted
- * using a given lock. This value is used to detect if there has been
- * important WAL activity since the last time some action, like a checkpoint,
- * was performed - allowing to not repeat the action if not. The LSN is
- * updated for all insertions, unless the XLOG_MARK_UNIMPORTANT flag was
- * set. lastImportantAt is never cleared, only overwritten by the LSN of newer
- * records.  Tracking the WAL activity directly in WALInsertLock has the
- * advantage of not needing any additional locks to update the value.
  */
 typedef struct
 {
 	LWLock		lock;
 	XLogRecPtr	insertingAt;
-	XLogRecPtr	lastImportantAt;
 } WALInsertLock;
 
 /*
@@ -447,6 +437,19 @@ typedef struct XLogCtlInsert
 	int			runningBackups;
 	XLogRecPtr	lastBackupStart;
 
+	slock_t		lastimportantpos_lck;	/* protects lastImportantPos */
+
+	/*
+	 * lastImportantPos contains the LSN of the last important WAL record
+	 * inserted in the server. This value is used to detect if there has been
+	 * important WAL activity since the last time some action, like a
+	 * checkpoint, was performed - allowing to not repeat the action if not.
+	 * The LSN is updated for all insertions, unless the XLOG_MARK_UNIMPORTANT
+	 * flag was set. lastImportantPos is never cleared, only overwritten by the
+	 * LSN of newer records.
+	 */
+	XLogRecPtr	lastImportantPos;
+
 	/*
 	 * WAL insertion locks.
 	 */
@@ -871,9 +874,9 @@ XLogInsertRecord(XLogRecData *rdata,
 		 */
 		if ((flags & XLOG_MARK_UNIMPORTANT) == 0)
 		{
-			int			lockno = holdingAllLocks ? 0 : MyLockNo;
-
-			WALInsertLocks[lockno].l.lastImportantAt = StartPos;
+			SpinLockAcquire(&Insert->lastimportantpos_lck);
+			Insert->lastImportantPos = StartPos;
+			SpinLockRelease(&Insert->lastimportantpos_lck);
 		}
 	}
 	else
@@ -4603,9 +4606,10 @@ XLOGShmemInit(void)
 	{
 		LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
 		WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr;
-		WALInsertLocks[i].l.lastImportantAt = InvalidXLogRecPtr;
 	}
 
+	XLogCtl->Insert.lastImportantPos = InvalidXLogRecPtr;
+
 	/*
 	 * Align the start of the page buffers to a full xlog block size boundary.
 	 * This simplifies some calculations in XLOG insertion. It is also
@@ -4625,6 +4629,7 @@ XLOGShmemInit(void)
 	XLogCtl->WalWriterSleeping = false;
 
 	SpinLockInit(&XLogCtl->Insert.insertpos_lck);
+	SpinLockInit(&XLogCtl->Insert.lastimportantpos_lck);
 	SpinLockInit(&XLogCtl->info_lck);
 	SpinLockInit(&XLogCtl->ulsn_lck);
 }
@@ -6109,32 +6114,16 @@ GetWALInsertionTimeLine(void)
  * GetLastImportantRecPtr -- Returns the LSN of the last important record
  * inserted. All records not explicitly marked as unimportant are considered
  * important.
- *
- * The LSN is determined by computing the maximum of
- * WALInsertLocks[i].lastImportantAt.
  */
 XLogRecPtr
 GetLastImportantRecPtr(void)
 {
+	XLogCtlInsert *Insert = &XLogCtl->Insert;
 	XLogRecPtr	res = InvalidXLogRecPtr;
-	int			i;
 
-	for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
-	{
-		XLogRecPtr	last_important;
-
-		/*
-		 * Need to take a lock to prevent torn reads of the LSN, which are
-		 * possible on some of the supported platforms. WAL insert locks only
-		 * support exclusive mode, so we have to use that.
-		 */
-		LWLockAcquire(&WALInsertLocks[i].l.lock, LW_EXCLUSIVE);
-		last_important = WALInsertLocks[i].l.lastImportantAt;
-		LWLockRelease(&WALInsertLocks[i].l.lock);
-
-		if (res < last_important)
-			res = last_important;
-	}
+	SpinLockAcquire(&Insert->lastimportantpos_lck);
+	res = Insert->lastImportantPos;
+	SpinLockRelease(&Insert->lastimportantpos_lck);
 
 	return res;
 }
-- 
2.34.1