From b3592b4c732b6439087090292fb63b6aca2a09b7 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev 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 Reviewed-by: Daniel Gustafsson Reviewed-by: Noah Misch Reviewed-by: Pavel Borisov Reviewed-by: Maxim Orlov 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