From 6f6d0389a599e88cf4af4be0e8d4ed3df26100a1 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev Date: Wed, 30 Mar 2022 15:17:10 +0300 Subject: [PATCH v5] Add unit tests for SLRU SimpleLruInit() checks whether it is called under postmaster. For this reason the tests can't be implemented in regress.c without changing the interface. We wanted to avoid this. As a result the tests were implemented as an extension in src/test/modules/ directory. Aleksander Alekseev, reviewed by Michael Paquier, Daniel Gustafsson, Noah Misch, Pavel Borisov, Maxim Orlov. Discussion: https://postgr.es/m/CAJ7c6TOFoWcHOW4BVe3BG_uikCrO9B91ayx9d6rh5JZr_tPESg%40mail.gmail.com --- src/test/modules/Makefile | 1 + src/test/modules/meson.build | 1 + src/test/modules/test_slru/.gitignore | 4 + src/test/modules/test_slru/Makefile | 23 ++ src/test/modules/test_slru/meson.build | 34 ++ src/test/modules/test_slru/t/001_base.pl | 33 ++ src/test/modules/test_slru/test_slru--1.0.sql | 5 + src/test/modules/test_slru/test_slru.c | 301 ++++++++++++++++++ src/test/modules/test_slru/test_slru.control | 4 + 9 files changed, 406 insertions(+) create mode 100644 src/test/modules/test_slru/.gitignore create mode 100644 src/test/modules/test_slru/Makefile create mode 100644 src/test/modules/test_slru/meson.build create mode 100644 src/test/modules/test_slru/t/001_base.pl create mode 100644 src/test/modules/test_slru/test_slru--1.0.sql create mode 100644 src/test/modules/test_slru/test_slru.c create mode 100644 src/test/modules/test_slru/test_slru.control diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index 7b3f292965..af3eef19b0 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -30,6 +30,7 @@ SUBDIRS = \ test_regex \ test_rls_hooks \ test_shm_mq \ + test_slru \ unsafe_tests \ worker_spi diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index c2e5f5ffd5..0b2dd3134a 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -24,5 +24,6 @@ subdir('test_rbtree') subdir('test_regex') subdir('test_rls_hooks') subdir('test_shm_mq') +subdir('test_slru') subdir('unsafe_tests') subdir('worker_spi') diff --git a/src/test/modules/test_slru/.gitignore b/src/test/modules/test_slru/.gitignore new file mode 100644 index 0000000000..4341ef65fe --- /dev/null +++ b/src/test/modules/test_slru/.gitignore @@ -0,0 +1,4 @@ +*.o +*.so +*.bc +results diff --git a/src/test/modules/test_slru/Makefile b/src/test/modules/test_slru/Makefile new file mode 100644 index 0000000000..10b79fb5aa --- /dev/null +++ b/src/test/modules/test_slru/Makefile @@ -0,0 +1,23 @@ +# src/test/modules/test_slru/Makefile + +MODULE_big = test_slru +OBJS = \ + $(WIN32RES) \ + test_slru.o +PGFILEDESC = "test_slru - example use of SLRU" + +EXTENSION = test_slru +DATA = test_slru--1.0.sql + +TAP_TESTS = 1 + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_slru +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_slru/meson.build b/src/test/modules/test_slru/meson.build new file mode 100644 index 0000000000..8a7386cea5 --- /dev/null +++ b/src/test/modules/test_slru/meson.build @@ -0,0 +1,34 @@ +# FIXME: prevent install during main install, but not during test :/ + +test_slru_sources = files( + 'test_slru.c', +) + +if host_system == 'windows' + test_slru_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_slru', + '--FILEDESC', 'test_slru - Test code for SLRU',]) +endif + +test_slru = shared_module('test_slru', + test_slru_sources, + kwargs: pg_mod_args, +) +testprep_targets += test_slru + +install_data( + 'test_slru.control', + 'test_slru--1.0.sql', + kwargs: contrib_data_args, +) + +tests += { + 'name': 'test_slru', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_base.pl', + ], + }, +} \ No newline at end of file diff --git a/src/test/modules/test_slru/t/001_base.pl b/src/test/modules/test_slru/t/001_base.pl new file mode 100644 index 0000000000..967ed400c0 --- /dev/null +++ b/src/test/modules/test_slru/t/001_base.pl @@ -0,0 +1,33 @@ + +# Copyright (c) 2022, PostgreSQL Global Development Group + +use strict; +use warnings; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $result; +my $cmdret; +my $node = PostgreSQL::Test::Cluster->new('main'); + +$node->init; +$node->append_conf('postgresql.conf', qq{shared_preload_libraries = 'test_slru'}); +$node->append_conf('postgresql.conf', qq{log_min_messages = 'NOTICE'}); +$node->start; + +$node->safe_psql("postgres", "CREATE EXTENSION test_slru;"); + +# The test is executed twice to make sure the state is cleaned up properly +$result = $node->safe_psql("postgres", "SELECT test_slru();"); +ok($result eq "", 'the message is empty'); + +$result = $node->safe_psql("postgres", "SELECT test_slru();"); +ok($result eq "", 'the message is empty'); + +$node->safe_psql("postgres", "DROP EXTENSION test_slru;"); + +$node->stop; + +done_testing(); diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql new file mode 100644 index 0000000000..f36cbf2f1f --- /dev/null +++ b/src/test/modules/test_slru/test_slru--1.0.sql @@ -0,0 +1,5 @@ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_slru" to load this file. \quit + +CREATE OR REPLACE FUNCTION test_slru() RETURNS VOID +AS 'MODULE_PATHNAME', 'test_slru' LANGUAGE C; diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c new file mode 100644 index 0000000000..15dd449e31 --- /dev/null +++ b/src/test/modules/test_slru/test_slru.c @@ -0,0 +1,301 @@ +/*-------------------------------------------------------------------------- + * + * test_slru.c + * Test correctness of SLRU functions. + * + * Copyright (c) 2022, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_slru/test_slru.c + * + * ------------------------------------------------------------------------- + */ + +#include +#include +#include +#include +#include +#include +#include "access/slru.h" +#include "access/transam.h" +#include "storage/fd.h" + +PG_MODULE_MAGIC; + +/* + * SQL-callable entry point to perform all the tests + */ +PG_FUNCTION_INFO_V1(test_slru); + + +#define NUM_TEST_BUFFERS 16 + +/* SLRU control lock */ +LWLock TestSLRULock; +#define TestSLRULock (&TestSLRULock) + +static SlruCtlData TestSlruCtlData; +#define TestSlruCtl (&TestSlruCtlData) + +static shmem_request_hook_type prev_shmem_request_hook = NULL; +static shmem_startup_hook_type prev_shmem_startup_hook = NULL; + +const char test_tranche_name[] = "test_slru_tranche"; +const char test_data[] = "Test data"; + +static int CallbackCounter; + +static void +EnsureStringEquals(const char *value, const char *expected) +{ + if (strcmp(value, expected) != 0) + elog(ERROR, "Got '%s', expected '%s'", value, expected); +} + +static void +EnsureIntEquals(int value, int expected) +{ + if (value != expected) + elog(ERROR, "Got %d, expected %d", value, expected); +} + +static bool +test_slru_page_precedes_logically(int page1, int page2) +{ + return page1 < page2; +} + +static bool +test_slru_scan_cb(SlruCtl ctl, char *filename, int segpage, void *data) +{ + /* + * Since the scan order is not guaranteed, we don't display these values. + * What actually interests us is the number of times the callback will be + * executed. + */ + (void) filename; + (void) segpage; + + elog(NOTICE, "test_slru_scan_cb() called, (char*))data = '%s'", (char *) data); + EnsureStringEquals(data, test_data); + + CallbackCounter++; + + return false; /* keep going */ +} + +static void +test_slru_shmem_request(void) +{ + if (prev_shmem_request_hook) + prev_shmem_request_hook(); + + /* Reserve shared memory for the test SLRU */ + RequestAddinShmemSpace(SimpleLruShmemSize(NUM_TEST_BUFFERS, 0)); +} + +static void +test_slru_shmem_startup(void) +{ + const char slru_dir_name[] = "pg_test_slru"; + int test_tranche_id; + + if (prev_shmem_startup_hook) + prev_shmem_startup_hook(); + + elog(NOTICE, "Creating directory '%s', if not exists", + slru_dir_name); + (void) MakePGDirectory(slru_dir_name); + + elog(NOTICE, "Initializing SLRU"); + + test_tranche_id = LWLockNewTrancheId(); + LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + LWLockInitialize(TestSLRULock, test_tranche_id); + + TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; + SimpleLruInit(TestSlruCtl, "Test", + NUM_TEST_BUFFERS, 0, TestSLRULock, slru_dir_name, + test_tranche_id, SYNC_HANDLER_NONE); + + elog(NOTICE, "SLRU initialized"); +} + +void +_PG_init(void) +{ + if (!process_shared_preload_libraries_in_progress) + elog(FATAL, "Please use shared_preload_libraries"); + + elog(LOG, "extension loaded"); + + prev_shmem_request_hook = shmem_request_hook; + shmem_request_hook = test_slru_shmem_request; + + prev_shmem_startup_hook = shmem_startup_hook; + shmem_startup_hook = test_slru_shmem_startup; +} + +Datum +test_slru(PG_FUNCTION_ARGS) +{ + bool found; + int slotno, + i; + FileTag ftag; + char path[MAXPGPATH]; + int test_pageno = 12345; + + 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_data); + + elog(NOTICE, "Success, shared->page_number[slotno] = %d", + TestSlruCtl->shared->page_number[slotno]); + EnsureIntEquals(TestSlruCtl->shared->page_number[slotno], test_pageno); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d. " + "Now writing the page.", found); + EnsureIntEquals(found, false); + + SimpleLruWritePage(TestSlruCtl, slotno); + LWLockRelease(TestSLRULock); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d", found); + EnsureIntEquals(found, true); + + elog(NOTICE, "Writing %d more pages...", NUM_TEST_BUFFERS * 3); + for (i = 1; i <= NUM_TEST_BUFFERS * 3; i++) + { + LWLockAcquire(TestSLRULock, LW_EXCLUSIVE); + slotno = SimpleLruZeroPage(TestSlruCtl, test_pageno + i); + + TestSlruCtl->shared->page_dirty[slotno] = true; + TestSlruCtl->shared->page_status[slotno] = SLRU_PAGE_VALID; + strcpy(TestSlruCtl->shared->page_buffer[slotno], test_data); + LWLockRelease(TestSLRULock); + } + + elog(NOTICE, "Reading page %d (in buffer) for read and write", + test_pageno + NUM_TEST_BUFFERS * 2); + LWLockAcquire(TestSLRULock, LW_EXCLUSIVE); + slotno = SimpleLruReadPage(TestSlruCtl, test_pageno + NUM_TEST_BUFFERS * 2, + true /* write_ok */ , InvalidTransactionId); + LWLockRelease(TestSLRULock); + + /* + * Same for SimpleLruReadPage_ReadOnly. Note that the control lock should + * NOT be held at entry, but will be held at exit. + */ + elog(NOTICE, "Reading page %d (in buffer) for read-only", + test_pageno + NUM_TEST_BUFFERS * 2); + slotno = SimpleLruReadPage_ReadOnly(TestSlruCtl, + test_pageno + NUM_TEST_BUFFERS * 2, InvalidTransactionId); + LWLockRelease(TestSLRULock); + + elog(NOTICE, "Reading page %d (not in buffer) for read and write", + test_pageno); + LWLockAcquire(TestSLRULock, LW_EXCLUSIVE); + slotno = SimpleLruReadPage(TestSlruCtl, test_pageno, + true /* write_ok */ , InvalidTransactionId); + LWLockRelease(TestSLRULock); + + /* + * Same for SimpleLruReadPage_ReadOnly. Note that the control lock should + * NOT be held at entry, but will be held at exit. + */ + elog(NOTICE, "Reading page %d (not in buffer) for read-only", + test_pageno + 1); + slotno = SimpleLruReadPage_ReadOnly(TestSlruCtl, + test_pageno + 1, InvalidTransactionId); + LWLockRelease(TestSLRULock); + + elog(NOTICE, "Calling SimpleLruWriteAll()"); + SimpleLruWriteAll(TestSlruCtl, true); + + ftag.segno = (test_pageno + NUM_TEST_BUFFERS * 3) / SLRU_PAGES_PER_SEGMENT; + elog(NOTICE, "Calling SlruSyncFileTag() for segment %d", ftag.segno); + SlruSyncFileTag(TestSlruCtl, &ftag, path); + elog(NOTICE, "Done, path = %s", path); + EnsureStringEquals(path, "pg_test_slru/0183"); + + /* Test SlruDeleteSegment() and SlruScanDirectory() */ + + /* check precondition */ + CallbackCounter = 0; + elog(NOTICE, "Calling SlruScanDirectory()"); + SlruScanDirectory(TestSlruCtl, test_slru_scan_cb, (void *) test_data); + EnsureIntEquals(CallbackCounter, 3); + + /* delete the segment */ + elog(NOTICE, "Deleting segment %d", ftag.segno); + SlruDeleteSegment(TestSlruCtl, ftag.segno); + + /* check postcondition */ + CallbackCounter = 0; + elog(NOTICE, "Calling SlruScanDirectory()"); + SlruScanDirectory(TestSlruCtl, test_slru_scan_cb, (void *) test_data); + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, + (test_pageno + NUM_TEST_BUFFERS * 3)); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d", found); + EnsureIntEquals(CallbackCounter, 2); + EnsureIntEquals(found, false); + + /* Test SimpleLruTruncate() */ + + /* check precondition */ + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, + test_pageno + NUM_TEST_BUFFERS * 2); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno + NUM_TEST_BUFFERS * 2); + EnsureIntEquals(found, true); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno); + EnsureIntEquals(found, true); + + /* do the call */ + elog(NOTICE, "Truncating pages prior to %d", + test_pageno + NUM_TEST_BUFFERS * 2); + SimpleLruTruncate(TestSlruCtl, test_pageno + NUM_TEST_BUFFERS * 2); + + /* check postcondition */ + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, + test_pageno + NUM_TEST_BUFFERS * 2); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno + NUM_TEST_BUFFERS * 2); + EnsureIntEquals(found, true); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno); + EnsureIntEquals(found, false); + + /* this is to cover SlruScanDirCbDeleteAll() with tests as well */ + CallbackCounter = 0; + elog(NOTICE, "Cleaning up."); + SlruScanDirectory(TestSlruCtl, SlruScanDirCbDeleteAll, NULL); + EnsureIntEquals(CallbackCounter, 0); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, + test_pageno + NUM_TEST_BUFFERS * 2); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno + NUM_TEST_BUFFERS * 2); + EnsureIntEquals(found, false); + + found = SimpleLruDoesPhysicalPageExist(TestSlruCtl, test_pageno); + elog(NOTICE, "SimpleLruDoesPhysicalPageExist() returned %d for page %d", + found, test_pageno); + EnsureIntEquals(found, false); + + PG_RETURN_VOID(); +} diff --git a/src/test/modules/test_slru/test_slru.control b/src/test/modules/test_slru/test_slru.control new file mode 100644 index 0000000000..742541b297 --- /dev/null +++ b/src/test/modules/test_slru/test_slru.control @@ -0,0 +1,4 @@ +comment = 'Test code for SLRU' +default_version = '1.0' +module_pathname = '$libdir/test_slru' +relocatable = false -- 2.38.1