Hello hackers!
The functions, bootstrapping SLRU pages, such as BootStrapMultiXact,
BootStrapCLOG, ActivateCommitTs, multixact_redo and others, have a lot
of repetitive code.
A new proposed function BootStrapSlruPage moves a duplicating code into
the single place. Additionally, a new member ZeroFunc is implemented in
the SlruCtlData structure. The ZeroFunc keeps a pointer to a function
proper for nullifying SLRU pages of a type defined by a corresponding
SlruCtlData object.
Applying proposed modifications can simplify maintenance and future
development of Postgres code in the realm of bootstrapping SLRU.
Best regards,
Evgeny Voropaev,
Tantor Labs LLC.
From 62762bf4a86af76d845755e41f46714d2b41bbe4 Mon Sep 17 00:00:00 2001
From: Evgeny Voropaev <evo...@gmail.com>
Date: Thu, 13 Feb 2025 12:43:20 +0800
Subject: [PATCH v1] Elimination of the repetitive code at the SLRU bootstrap
functions.
The functions bootstrapping SLRU pages had a lot of repetitive code. A new
realized function BootStrapSlruPage has moved a duplicating code into the
single place. Additionally, a new member ZeroFunc has been implemented in the
SlruCtlData structure. The ZeroFunc keeps a pointer to a function proper
for nullifying SLRU pages of a type defined by a corresponding SlruCtlData
object.
Author: Evgeny Voropaev <evgeny.vorop...@tantorlabs.com> <evo...@gmail.com>
---
src/backend/access/transam/clog.c | 43 ++++------------
src/backend/access/transam/commit_ts.c | 38 ++++----------
src/backend/access/transam/multixact.c | 71 +++++---------------------
src/backend/access/transam/slru.c | 34 ++++++++++++
src/backend/access/transam/subtrans.c | 35 +++++--------
src/include/access/slru.h | 8 +++
6 files changed, 87 insertions(+), 142 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 0d556c00b8c..d02a80378a1 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -102,14 +102,6 @@ TransactionIdToPage(TransactionId xid)
*/
#define THRESHOLD_SUBTRANS_CLOG_OPT 5
-/*
- * Link to shared-memory data structures for CLOG control
- */
-static SlruCtlData XactCtlData;
-
-#define XactCtl (&XactCtlData)
-
-
static int ZeroCLOGPage(int64 pageno, bool writeXlog);
static bool CLOGPagePrecedes(int64 page1, int64 page2);
static void WriteZeroPageXlogRec(int64 pageno);
@@ -130,6 +122,14 @@ static void TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
XLogRecPtr lsn, int64 pageno);
+/*
+ * Link to shared-memory data structures for CLOG control
+ */
+static SlruCtlData XactCtlData = { .ZeroPage = ZeroCLOGPage };
+
+#define XactCtl (&XactCtlData)
+
+
/*
* TransactionIdSetTreeStatus
*
@@ -832,19 +832,7 @@ check_transaction_buffers(int *newval, void **extra, GucSource source)
void
BootStrapCLOG(void)
{
- int slotno;
- LWLock *lock = SimpleLruGetBankLock(XactCtl, 0);
-
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- /* Create and zero the first page of the commit log */
- slotno = ZeroCLOGPage(0, false);
-
- /* Make sure it's written out */
- SimpleLruWritePage(XactCtl, slotno);
- Assert(!XactCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(XactCtl, 0);
}
/*
@@ -1114,19 +1102,8 @@ clog_redo(XLogReaderState *record)
if (info == CLOG_ZEROPAGE)
{
int64 pageno;
- int slotno;
- LWLock *lock;
-
memcpy(&pageno, XLogRecGetData(record), sizeof(pageno));
-
- lock = SimpleLruGetBankLock(XactCtl, pageno);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- slotno = ZeroCLOGPage(pageno, false);
- SimpleLruWritePage(XactCtl, slotno);
- Assert(!XactCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(XactCtl, pageno);
}
else if (info == CLOG_TRUNCATE)
{
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 95049acd0b5..33ef6716535 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -77,13 +77,6 @@ TransactionIdToCTsPage(TransactionId xid)
#define TransactionIdToCTsEntry(xid) \
((xid) % (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
-/*
- * Link to shared-memory data structures for CommitTs control
- */
-static SlruCtlData CommitTsCtlData;
-
-#define CommitTsCtl (&CommitTsCtlData)
-
/*
* We keep a cache of the last value set in shared memory.
*
@@ -121,6 +114,13 @@ static void DeactivateCommitTs(void);
static void WriteZeroPageXlogRec(int64 pageno);
static void WriteTruncateXlogRec(int64 pageno, TransactionId oldestXid);
+/*
+ * Link to shared-memory data structures for CommitTs control
+ */
+static SlruCtlData CommitTsCtlData = { .ZeroPage = ZeroCommitTsPage };
+
+#define CommitTsCtl (&CommitTsCtlData)
+
/*
* TransactionTreeSetCommitTsData
*
@@ -747,16 +747,7 @@ ActivateCommitTs(void)
/* Create the current segment file, if necessary */
if (!SimpleLruDoesPhysicalPageExist(CommitTsCtl, pageno))
- {
- LWLock *lock = SimpleLruGetBankLock(CommitTsCtl, pageno);
- int slotno;
-
- LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = ZeroCommitTsPage(pageno, false);
- SimpleLruWritePage(CommitTsCtl, slotno);
- Assert(!CommitTsCtl->shared->page_dirty[slotno]);
- LWLockRelease(lock);
- }
+ BootStrapSlruPage(CommitTsCtl, pageno);
/* Change the activation status in shared memory. */
LWLockAcquire(CommitTsLock, LW_EXCLUSIVE);
@@ -1023,19 +1014,8 @@ commit_ts_redo(XLogReaderState *record)
if (info == COMMIT_TS_ZEROPAGE)
{
int64 pageno;
- int slotno;
- LWLock *lock;
-
memcpy(&pageno, XLogRecGetData(record), sizeof(pageno));
-
- lock = SimpleLruGetBankLock(CommitTsCtl, pageno);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- slotno = ZeroCommitTsPage(pageno, false);
- SimpleLruWritePage(CommitTsCtl, slotno);
- Assert(!CommitTsCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(CommitTsCtl, pageno);
}
else if (info == COMMIT_TS_TRUNCATE)
{
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 27ccdf9500f..5fb688d19a1 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -223,15 +223,6 @@ PreviousMultiXactId(MultiXactId multi)
return multi == FirstMultiXactId ? MaxMultiXactId : multi - 1;
}
-/*
- * Links to shared-memory data structures for MultiXact control
- */
-static SlruCtlData MultiXactOffsetCtlData;
-static SlruCtlData MultiXactMemberCtlData;
-
-#define MultiXactOffsetCtl (&MultiXactOffsetCtlData)
-#define MultiXactMemberCtl (&MultiXactMemberCtlData)
-
/*
* MultiXact state shared across all backends. All this state is protected
* by MultiXactGenLock. (We also use SLRU bank's lock of MultiXactOffset and
@@ -420,6 +411,14 @@ static void WriteMTruncateXlogRec(Oid oldestMultiDB,
MultiXactOffset startTruncMemb,
MultiXactOffset endTruncMemb);
+/*
+ * Links to shared-memory data structures for MultiXact control
+ */
+static SlruCtlData MultiXactOffsetCtlData = { .ZeroPage = &ZeroMultiXactOffsetPage };
+static SlruCtlData MultiXactMemberCtlData = { .ZeroPage = &ZeroMultiXactMemberPage };
+
+#define MultiXactOffsetCtl (&MultiXactOffsetCtlData)
+#define MultiXactMemberCtl (&MultiXactMemberCtlData)
/*
* MultiXactIdCreate
@@ -2033,32 +2032,8 @@ check_multixact_member_buffers(int *newval, void **extra, GucSource source)
void
BootStrapMultiXact(void)
{
- int slotno;
- LWLock *lock;
-
- lock = SimpleLruGetBankLock(MultiXactOffsetCtl, 0);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- /* Create and zero the first page of the offsets log */
- slotno = ZeroMultiXactOffsetPage(0, false);
-
- /* Make sure it's written out */
- SimpleLruWritePage(MultiXactOffsetCtl, slotno);
- Assert(!MultiXactOffsetCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
-
- lock = SimpleLruGetBankLock(MultiXactMemberCtl, 0);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- /* Create and zero the first page of the members log */
- slotno = ZeroMultiXactMemberPage(0, false);
-
- /* Make sure it's written out */
- SimpleLruWritePage(MultiXactMemberCtl, slotno);
- Assert(!MultiXactMemberCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(MultiXactOffsetCtl, 0);
+ BootStrapSlruPage(MultiXactMemberCtl, 0);
}
/*
@@ -3401,36 +3376,14 @@ multixact_redo(XLogReaderState *record)
if (info == XLOG_MULTIXACT_ZERO_OFF_PAGE)
{
int64 pageno;
- int slotno;
- LWLock *lock;
-
memcpy(&pageno, XLogRecGetData(record), sizeof(pageno));
-
- lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- slotno = ZeroMultiXactOffsetPage(pageno, false);
- SimpleLruWritePage(MultiXactOffsetCtl, slotno);
- Assert(!MultiXactOffsetCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(MultiXactOffsetCtl, pageno);
}
else if (info == XLOG_MULTIXACT_ZERO_MEM_PAGE)
{
int64 pageno;
- int slotno;
- LWLock *lock;
-
memcpy(&pageno, XLogRecGetData(record), sizeof(pageno));
-
- lock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- slotno = ZeroMultiXactMemberPage(pageno, false);
- SimpleLruWritePage(MultiXactMemberCtl, slotno);
- Assert(!MultiXactMemberCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(MultiXactMemberCtl, pageno);
}
else if (info == XLOG_MULTIXACT_CREATE_ID)
{
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 9ce628e62a5..b73bea16972 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -1850,3 +1850,37 @@ SlruSyncFileTag(SlruCtl ctl, const FileTag *ftag, char *path)
errno = save_errno;
return result;
}
+
+/*
+ * BootStrapSlruPage is a workhorse for functions bootstraping
+ * SLRU pages, including:
+ * BootStrapMultiXact,
+ * BootStrapCLOG,
+ * ActivateCommitTs,
+ * multixact_redo,
+ * commit_ts_redo.
+ *
+ * It performs all of the rut such as acquiring a lock, zeroing,
+ * ensuring that bootstrapped page is written out, and releasing the lock.
+ *
+ * A caller has to provide a proper ZeroFunc in the ctl structure.
+ *
+ */
+void
+BootStrapSlruPage(SlruCtl ctl, int64 pageno)
+{
+ int slotno;
+ LWLock *lock;
+
+ lock = SimpleLruGetBankLock(ctl, pageno);
+ LWLockAcquire(lock, LW_EXCLUSIVE);
+
+ /* Create and zero the page in SLRU */
+ slotno = ctl->ZeroPage(pageno, false);
+
+ /* Make sure it's written out */
+ SimpleLruWritePage(ctl, slotno);
+ Assert(!ctl->shared->page_dirty[slotno]);
+
+ LWLockRelease(lock);
+}
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 15153618fad..bad2c15951b 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -66,18 +66,18 @@ TransactionIdToPage(TransactionId xid)
#define TransactionIdToEntry(xid) ((xid) % (TransactionId) SUBTRANS_XACTS_PER_PAGE)
+static int ZeroSUBTRANSPage(int64 pageno, bool unused);
+static bool SubTransPagePrecedes(int64 page1, int64 page2);
+
+
/*
* Link to shared-memory data structures for SUBTRANS control
*/
-static SlruCtlData SubTransCtlData;
+static SlruCtlData SubTransCtlData = { .ZeroPage = ZeroSUBTRANSPage };
#define SubTransCtl (&SubTransCtlData)
-static int ZeroSUBTRANSPage(int64 pageno);
-static bool SubTransPagePrecedes(int64 page1, int64 page2);
-
-
/*
* Record the parent of a subtransaction in the subtrans log.
*/
@@ -269,19 +269,7 @@ check_subtrans_buffers(int *newval, void **extra, GucSource source)
void
BootStrapSUBTRANS(void)
{
- int slotno;
- LWLock *lock = SimpleLruGetBankLock(SubTransCtl, 0);
-
- LWLockAcquire(lock, LW_EXCLUSIVE);
-
- /* Create and zero the first page of the subtrans log */
- slotno = ZeroSUBTRANSPage(0);
-
- /* Make sure it's written out */
- SimpleLruWritePage(SubTransCtl, slotno);
- Assert(!SubTransCtl->shared->page_dirty[slotno]);
-
- LWLockRelease(lock);
+ BootStrapSlruPage(SubTransCtl, 0);
}
/*
@@ -291,10 +279,15 @@ BootStrapSUBTRANS(void)
* The slot number of the new page is returned.
*
* Control lock must be held at entry, and will be held at exit.
+ *
+ * bool unused is a parameter required to provide matching of the
+ * ZeroSUBTRANSPage function to the SlruCtlData.ZeroPage function type.
*/
static int
-ZeroSUBTRANSPage(int64 pageno)
+ZeroSUBTRANSPage(int64 pageno, bool unused)
{
+ (void)unused; /* Supress compiler warning*/
+
return SimpleLruZeroPage(SubTransCtl, pageno);
}
@@ -335,7 +328,7 @@ StartupSUBTRANS(TransactionId oldestActiveXID)
prevlock = lock;
}
- (void) ZeroSUBTRANSPage(startPage);
+ (void) ZeroSUBTRANSPage(startPage, false);
if (startPage == endPage)
break;
@@ -395,7 +388,7 @@ ExtendSUBTRANS(TransactionId newestXact)
LWLockAcquire(lock, LW_EXCLUSIVE);
/* Zero the page */
- ZeroSUBTRANSPage(pageno);
+ ZeroSUBTRANSPage(pageno, false);
LWLockRelease(lock);
}
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index e142800aab2..ec48355ac74 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -161,6 +161,13 @@ typedef struct SlruCtlData
* it's always the same, it doesn't need to be in shared memory.
*/
char Dir[64];
+
+ /*
+ * Function for nulifying a page pointed by this SlruCtlData object
+ * Signature:
+ * int ZeroPage(int64 pageno, bool writeXlog);
+ */
+ int (*ZeroPage) (int64, bool);
} SlruCtlData;
typedef SlruCtlData *SlruCtl;
@@ -214,5 +221,6 @@ extern bool SlruScanDirCbReportPresence(SlruCtl ctl, char *filename,
extern bool SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage,
void *data);
extern bool check_slru_buffers(const char *name, int *newval);
+extern void BootStrapSlruPage(SlruCtl ctl, int64 pageno);
#endif /* SLRU_H */
--
2.47.1
