v2-0001-Unit-tests-for-SLRU.patch

From b3592b4c732b6439087090292fb63b6aca2a09b7 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 30 Mar 2022 15:17:10 +0300
Subject: [PATCH v2] Unit tests for SLRU

This patch adds unit tests for SLRU. Also it moves Asserts() outside of
SimpleLruInit() in order to simplify the testing.

Author: Aleksander Alekxeev <aleksander@timescale.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Reviewed-by: Maxim Orlov <orlovmg@gmail.com>
Discussion: https://postgr.es/m/CAJ7c6TOFoWcHOW4BVe3BG_uikCrO9B91ayx9d6rh5JZr_tPESg%40mail.gmail.com
---
 src/backend/access/transam/clog.c      |  5 +-
 src/backend/access/transam/commit_ts.c |  4 +-
 src/backend/access/transam/multixact.c |  8 ++-
 src/backend/access/transam/slru.c      | 13 ++---
 src/backend/access/transam/subtrans.c  |  6 +-
 src/backend/commands/async.c           |  5 +-
 src/backend/storage/lmgr/predicate.c   |  5 +-
 src/include/access/slru.h              |  2 +-
 src/test/regress/expected/slru.out     | 38 ++++++++++++
 src/test/regress/parallel_schedule     |  2 +-
 src/test/regress/regress.c             | 80 ++++++++++++++++++++++++++
 src/test/regress/sql/slru.sql          | 16 ++++++
 12 files changed, 166 insertions(+), 18 deletions(-)
 create mode 100644 src/test/regress/expected/slru.out
 create mode 100644 src/test/regress/sql/slru.sql

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 3d9088a704..d95aad9178 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -696,10 +696,13 @@ CLOGShmemSize(void)
 void
 CLOGShmemInit(void)
 {
+	bool found;
+
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "Xact", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  XactSLRULock, "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  SYNC_HANDLER_CLOG);
+				  SYNC_HANDLER_CLOG, &found);
+	Assert(found == IsUnderPostmaster);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 20950eb1e4..69e016ab4c 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -544,7 +544,9 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "CommitTs", CommitTsShmemBuffers(), 0,
 				  CommitTsSLRULock, "pg_commit_ts",
 				  LWTRANCHE_COMMITTS_BUFFER,
-				  SYNC_HANDLER_COMMIT_TS);
+				  SYNC_HANDLER_COMMIT_TS,
+				  &found);
+	Assert(found == IsUnderPostmaster);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 9f65c600d0..4d8b98801e 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1866,13 +1866,17 @@ MultiXactShmemInit(void)
 				  "MultiXactOffset", NUM_MULTIXACTOFFSET_BUFFERS, 0,
 				  MultiXactOffsetSLRULock, "pg_multixact/offsets",
 				  LWTRANCHE_MULTIXACTOFFSET_BUFFER,
-				  SYNC_HANDLER_MULTIXACT_OFFSET);
+				  SYNC_HANDLER_MULTIXACT_OFFSET,
+				  &found);
+	Assert(found == IsUnderPostmaster);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "MultiXactMember", NUM_MULTIXACTMEMBER_BUFFERS, 0,
 				  MultiXactMemberSLRULock, "pg_multixact/members",
 				  LWTRANCHE_MULTIXACTMEMBER_BUFFER,
-				  SYNC_HANDLER_MULTIXACT_MEMBER);
+				  SYNC_HANDLER_MULTIXACT_MEMBER,
+				  &found);
+	Assert(found == IsUnderPostmaster);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 30a476ed5d..949cf7c2a8 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -182,28 +182,27 @@ SimpleLruShmemSize(int nslots, int nlsns)
  * ctllock: LWLock to use to control access to the shared control structure.
  * subdir: PGDATA-relative subdirectory that will contain the files.
  * tranche_id: LWLock tranche ID to use for the SLRU's per-buffer LWLocks.
+ * sync_handler: see comments for SyncRequestHandler enum
+ * found: whether the cache existed before the call
  */
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  LWLock *ctllock, const char *subdir, int tranche_id,
-			  SyncRequestHandler sync_handler)
+			  SyncRequestHandler sync_handler, bool *found)
 {
 	SlruShared	shared;
-	bool		found;
 
 	shared = (SlruShared) ShmemInitStruct(name,
 										  SimpleLruShmemSize(nslots, nlsns),
-										  &found);
+										  found);
 
-	if (!IsUnderPostmaster)
+	if (!*found)
 	{
 		/* Initialize locks and shared memory area */
 		char	   *ptr;
 		Size		offset;
 		int			slotno;
 
-		Assert(!found);
-
 		memset(shared, 0, sizeof(SlruSharedData));
 
 		shared->ControlLock = ctllock;
@@ -256,8 +255,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 		/* Should fit to estimated shmem size */
 		Assert(ptr - (char *) shared <= SimpleLruShmemSize(nslots, nlsns));
 	}
-	else
-		Assert(found);
 
 	/*
 	 * Initialize the unshared control struct, including directory path. We
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 66d3548155..dbbd4d6e1d 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -31,6 +31,7 @@
 #include "access/slru.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "miscadmin.h"
 #include "pg_trace.h"
 #include "utils/snapmgr.h"
 
@@ -190,10 +191,13 @@ SUBTRANSShmemSize(void)
 void
 SUBTRANSShmemInit(void)
 {
+	bool found;
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "Subtrans", NUM_SUBTRANS_BUFFERS, 0,
 				  SubtransSLRULock, "pg_subtrans",
-				  LWTRANCHE_SUBTRANS_BUFFER, SYNC_HANDLER_NONE);
+				  LWTRANCHE_SUBTRANS_BUFFER, SYNC_HANDLER_NONE,
+				  &found);
+	Assert(found == IsUnderPostmaster);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 455d895a44..3fde060e2d 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -532,7 +532,7 @@ AsyncShmemSize(void)
 void
 AsyncShmemInit(void)
 {
-	bool		found;
+	bool		found, slru_found;
 	Size		size;
 	int			max_backends = GetMaxBackends();
 
@@ -572,7 +572,8 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "Notify", NUM_NOTIFY_BUFFERS, 0,
 				  NotifySLRULock, "pg_notify", LWTRANCHE_NOTIFY_BUFFER,
-				  SYNC_HANDLER_NONE);
+				  SYNC_HANDLER_NONE, &slru_found);
+	Assert(slru_found == IsUnderPostmaster);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index e337aad5b2..3a2e3aaad2 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -873,7 +873,10 @@ SerialInit(void)
 	SerialSlruCtl->PagePrecedes = SerialPagePrecedesLogically;
 	SimpleLruInit(SerialSlruCtl, "Serial",
 				  NUM_SERIAL_BUFFERS, 0, SerialSLRULock, "pg_serial",
-				  LWTRANCHE_SERIAL_BUFFER, SYNC_HANDLER_NONE);
+				  LWTRANCHE_SERIAL_BUFFER, SYNC_HANDLER_NONE,
+				  &found);
+	Assert(found == IsUnderPostmaster);
+
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 130c41c863..0bc20a0636 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -142,7 +142,7 @@ typedef SlruCtlData *SlruCtl;
 extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  LWLock *ctllock, const char *subdir, int tranche_id,
-						  SyncRequestHandler sync_handler);
+						  SyncRequestHandler sync_handler, bool *found);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
 							  TransactionId xid);
diff --git a/src/test/regress/expected/slru.out b/src/test/regress/expected/slru.out
new file mode 100644
index 0000000000..2e85cc1725
--- /dev/null
+++ b/src/test/regress/expected/slru.out
@@ -0,0 +1,38 @@
+--
+-- Unit tests for slru.c
+--
+-- directory paths and dlsuffix are passed to us in environment variables
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
+CREATE FUNCTION test_slru() RETURNS VOID
+    AS :'regresslib', 'test_slru' LANGUAGE C STRICT;
+-- The test is executed twice to make sure the state is cleaned up properly.
+SELECT test_slru();
+NOTICE:  Creating TestSLRULock...
+NOTICE:  Creating directory 'pg_test_slru', if not exists...
+NOTICE:  Creating SLRU, if not exists...
+NOTICE:  Calling SimpleLruZeroPage()...
+NOTICE:  Success, slotno = 0, shared->page_number[0] = 12345
+NOTICE:  SimpleLruDoesPhysicalPageExist() returned 0. Now writing the page.
+NOTICE:  SimpleLruDoesPhysicalPageExist() returned 1
+NOTICE:  Cleanig up...
+ test_slru 
+-----------
+ 
+(1 row)
+
+SELECT test_slru();
+NOTICE:  Creating TestSLRULock...
+NOTICE:  Creating directory 'pg_test_slru', if not exists...
+NOTICE:  Creating SLRU, if not exists...
+NOTICE:  Calling SimpleLruZeroPage()...
+NOTICE:  Success, slotno = 0, shared->page_number[0] = 12345
+NOTICE:  SimpleLruDoesPhysicalPageExist() returned 0. Now writing the page.
+NOTICE:  SimpleLruDoesPhysicalPageExist() returned 1
+NOTICE:  Cleanig up...
+ test_slru 
+-----------
+ 
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 5030d19c03..f0d01d6c59 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -33,7 +33,7 @@ test: strings numerology point lseg line box path polygon circle date time timet
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex slru type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 0802fb9136..f5029d18bf 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -21,6 +21,7 @@
 
 #include "access/detoast.h"
 #include "access/htup_details.h"
+#include "access/slru.h"
 #include "access/transam.h"
 #include "access/xact.h"
 #include "catalog/namespace.h"
@@ -38,6 +39,7 @@
 #include "optimizer/plancat.h"
 #include "parser/parse_coerce.h"
 #include "port/atomics.h"
+#include "storage/fd.h"
 #include "storage/spin.h"
 #include "utils/builtins.h"
 #include "utils/geo_decls.h"
@@ -1216,3 +1218,81 @@ binary_coercible(PG_FUNCTION_ARGS)
 
 	PG_RETURN_BOOL(IsBinaryCoercible(srctype, targettype));
 }
+
+/*
+ * Unit tests for slru.c
+ */
+
+#define NUM_TEST_BUFFERS		16
+
+int test_tranche_id = -1;
+LWLock TestSLRULock;
+#define TestSLRULock (&TestSLRULock)
+
+static SlruCtlData TestSlruCtlData;
+#define TestSlruCtl			(&TestSlruCtlData)
+
+static bool
+TestPagePrecedesLogically(int page1, int page2)
+{
+	return page1 < page2;
+}
+
+PG_FUNCTION_INFO_V1(test_slru);
+Datum
+test_slru(PG_FUNCTION_ARGS)
+{
+	bool found;
+	int slotno;
+	int test_pageno = 12345;
+	char test_page_data[] = "Test page data";
+	const char slru_dir_name[] = "pg_test_slru";
+
+	elog(NOTICE, "Creating TestSLRULock...");
+
+	test_tranche_id = LWLockNewTrancheId();
+	LWLockRegisterTranche(test_tranche_id, "test_slru_tranche");
+	LWLockInitialize(TestSLRULock, test_tranche_id);
+
+	elog(NOTICE, "Creating directory '%s', if not exists...",
+		slru_dir_name);
+	(void)MakePGDirectory(slru_dir_name);
+
+	elog(NOTICE, "Creating SLRU, if not exists...");
+
+	TestSlruCtl->PagePrecedes = TestPagePrecedesLogically;
+	SimpleLruInit(TestSlruCtl, "Test",
+				  NUM_TEST_BUFFERS, 0, TestSLRULock, slru_dir_name,
+				  test_tranche_id, SYNC_HANDLER_NONE, &found);
+
+	elog(NOTICE, "Calling SimpleLruZeroPage()...");
+
+	LWLockAcquire(TestSLRULock, LW_EXCLUSIVE);
+	slotno = SimpleLruZeroPage(TestSlruCtl, test_pageno);
+
+	TestSlruCtl->shared->page_dirty[slotno] = true;
+	TestSlruCtl->shared->page_status[slotno] = SLRU_PAGE_VALID;
+	strcpy(TestSlruCtl->shared->page_buffer[slotno], test_page_data);
+
+	elog(NOTICE, "Success, slotno = %d, shared->page_number[%d] = %d",
+		slotno, slotno, TestSlruCtl->shared->page_number[slotno]);
+
+	found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno);
+	elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d. "
+		"Now writing the page.", found);
+
+	/* Should we rename it to SimpleLruWriteSlot? -- a.alekseev */
+	SimpleLruWritePage(TestSlruCtl, slotno);
+	LWLockRelease(TestSLRULock);
+
+	found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno);
+	elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d", found);
+
+	elog(NOTICE, "Cleanig up...");
+	// SimpleLruWriteAll(TestSlruCtl, true);
+	// SimpleLruTruncate(TestSlruCtl, 555555);
+	SlruDeleteSegment(TestSlruCtl, test_pageno / SLRU_PAGES_PER_SEGMENT);
+
+	// TODO: test SlruReportIOError
+	PG_RETURN_VOID();
+}
diff --git a/src/test/regress/sql/slru.sql b/src/test/regress/sql/slru.sql
new file mode 100644
index 0000000000..83c9d5265f
--- /dev/null
+++ b/src/test/regress/sql/slru.sql
@@ -0,0 +1,16 @@
+--
+-- Unit tests for slru.c
+--
+
+-- directory paths and dlsuffix are passed to us in environment variables
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+
+\set regresslib :libdir '/regress' :dlsuffix
+
+CREATE FUNCTION test_slru() RETURNS VOID
+    AS :'regresslib', 'test_slru' LANGUAGE C STRICT;
+
+-- The test is executed twice to make sure the state is cleaned up properly.
+SELECT test_slru();
+SELECT test_slru();
\ No newline at end of file
-- 
2.35.1