From 651bdc4cab36e5294318e5a601688308c086984e Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Mon, 20 May 2024 15:20:31 +0500 Subject: [PATCH vAug2] Add multixact CV sleep test Previously we used 1ms sleep when we are reading multixact before another multixact without offset yet. a0e0fb1ba changed this behavior to CV sleep. This commit adds test for this edge case. To test such race condition we have to use waiting injection point under critical section. Such point requires special injection point loading. Author: Andrey Borodin Reviewed-by: Michael Paquier --- src/backend/access/transam/multixact.c | 7 ++ src/test/modules/test_slru/Makefile | 4 + src/test/modules/test_slru/meson.build | 8 ++ src/test/modules/test_slru/t/001_multixact.pl | 91 +++++++++++++++++++ src/test/modules/test_slru/test_slru--1.0.sql | 6 ++ src/test/modules/test_slru/test_slru.c | 38 ++++++++ 6 files changed, 154 insertions(+) create mode 100644 src/test/modules/test_slru/t/001_multixact.pl diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index c601ff98a1..898ea8f4c9 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -88,6 +88,7 @@ #include "storage/proc.h" #include "storage/procarray.h" #include "utils/fmgrprotos.h" +#include "utils/injection_point.h" #include "utils/guc_hooks.h" #include "utils/memutils.h" @@ -866,8 +867,12 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members) * in vacuum. During vacuum, in particular, it would be unacceptable to * keep OldestMulti set, in case it runs for long. */ + INJECTION_POINT_LOAD("get-new-multixact-id"); + multi = GetNewMultiXactId(nmembers, &offset); + INJECTION_POINT_CACHED("get-new-multixact-id"); + /* Make an XLOG entry describing the new MXID. */ xlrec.mid = multi; xlrec.moff = offset; @@ -1480,6 +1485,8 @@ retry: LWLockRelease(lock); CHECK_FOR_INTERRUPTS(); + INJECTION_POINT("get-multixact-member-cv-sleep"); + ConditionVariableSleep(&MultiXactState->nextoff_cv, WAIT_EVENT_MULTIXACT_CREATION); slept = true; diff --git a/src/test/modules/test_slru/Makefile b/src/test/modules/test_slru/Makefile index 936886753b..8f9623b128 100644 --- a/src/test/modules/test_slru/Makefile +++ b/src/test/modules/test_slru/Makefile @@ -6,6 +6,10 @@ OBJS = \ test_slru.o PGFILEDESC = "test_slru - test module for SLRUs" +EXTRA_INSTALL=src/test/modules/injection_points +export enable_injection_points enable_injection_points +TAP_TESTS = 1 + EXTENSION = test_slru DATA = test_slru--1.0.sql diff --git a/src/test/modules/test_slru/meson.build b/src/test/modules/test_slru/meson.build index ce91e60631..4a5bc6349a 100644 --- a/src/test/modules/test_slru/meson.build +++ b/src/test/modules/test_slru/meson.build @@ -32,4 +32,12 @@ tests += { 'regress_args': ['--temp-config', files('test_slru.conf')], 'runningcheck': false, }, + 'tap': { + 'env': { + 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no', + }, + 'tests': [ + 't/001_multixact.pl' + ], + }, } diff --git a/src/test/modules/test_slru/t/001_multixact.pl b/src/test/modules/test_slru/t/001_multixact.pl new file mode 100644 index 0000000000..b3d262e3c1 --- /dev/null +++ b/src/test/modules/test_slru/t/001_multixact.pl @@ -0,0 +1,91 @@ +# Copyright (c) 2024, PostgreSQL Global Development Group + +# This test verifies edge case of reading a multixact: +# when we have multixact that is followed by exactly one another multixact, +# and another multixact have no offset yet, we must wait until this offset +# becomes observable. Previously we used to wait for 1ms in a loop in this +# case, but now we use CV for this. This test is exercising such a sleep. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; + +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my ($node, $result); + +$node = PostgreSQL::Test::Cluster->new('multixact_CV_sleep'); +$node->init; +$node->append_conf('postgresql.conf', + "shared_preload_libraries = 'test_slru'"); +$node->start; +$node->safe_psql('postgres', q(CREATE EXTENSION injection_points)); +$node->safe_psql('postgres', q(CREATE EXTENSION test_slru)); + +# Test for Multixact generation edge case +$node->safe_psql('postgres', q(select injection_points_attach('test-read-multixact','wait'))); +$node->safe_psql('postgres', q(select injection_points_attach('get-multixact-member-cv-sleep','wait'))); + +# This session must observe sleep on CV when generating multixact. +# To achieve this it first will create a multixact, then pause before reading it. +my $observer = $node->background_psql('postgres'); + +# This query will create multixact, and hand just before reading it. +$observer->query_until(qr/start/, +q( + \echo start + select test_read_multixact(test_create_multixact()); +)); +$node->wait_for_event('client backend', 'test-read-multixact'); + +# This session will create next Multixact, it's necessary to avoid edge case 1 +# (see multixact.c) +my $creator = $node->background_psql('postgres'); +$node->safe_psql('postgres', q(select injection_points_attach('get-new-multixact-id','wait');)); + +# We expect this query to hand in critical section after generating new multixact, +# but before filling it's offset into SLRU. +# Running injection point under SLRU requires special loading of cache. +$creator->query_until(qr/start/, q( + \echo start + select test_create_multixact(); +)); + +$node->wait_for_event('client backend', 'get-new-multixact-id'); + +# Now we are sure we can reach edge case 2. +# Observer is going to read multixact, which has next, but next lacks offset. +$node->safe_psql('postgres', q(select injection_points_wakeup('test-read-multixact'))); + + +$node->wait_for_event('client backend', 'get-multixact-member-cv-sleep'); + +# Now we have two backends waiting in get-new-multixact-id and +# get-multixact-member-cv-sleep. Also we have 3 injections points set to wait. +# If we wakeup get-multixact-member-cv-sleep it will happen again, so we must +# detach it first. So let's detach all injection points, then wake up all +# backends. + +$node->safe_psql('postgres', q(select injection_points_detach('test-read-multixact'))); +$node->safe_psql('postgres', q(select injection_points_detach('get-new-multixact-id'))); +$node->safe_psql('postgres', q(select injection_points_detach('get-multixact-member-cv-sleep'))); + +$node->safe_psql('postgres', q(select injection_points_wakeup('get-new-multixact-id'))); +$node->safe_psql('postgres', q(select injection_points_wakeup('get-multixact-member-cv-sleep'))); + +# Background psql will now be able to read the result and disconnect. +$observer->quit; +$creator->quit; + +$node->stop; + +# If we reached this point - everything is OK. +ok(1); +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 index 202e8da3fd..58300c59a7 100644 --- a/src/test/modules/test_slru/test_slru--1.0.sql +++ b/src/test/modules/test_slru/test_slru--1.0.sql @@ -19,3 +19,9 @@ CREATE OR REPLACE FUNCTION test_slru_page_truncate(bigint) RETURNS VOID AS 'MODULE_PATHNAME', 'test_slru_page_truncate' LANGUAGE C; CREATE OR REPLACE FUNCTION test_slru_delete_all() RETURNS VOID AS 'MODULE_PATHNAME', 'test_slru_delete_all' LANGUAGE C; + + +CREATE OR REPLACE FUNCTION test_create_multixact() RETURNS xid + AS 'MODULE_PATHNAME', 'test_create_multixact' LANGUAGE C; +CREATE OR REPLACE FUNCTION test_read_multixact(xid) RETURNS VOID + AS 'MODULE_PATHNAME', 'test_read_multixact'LANGUAGE C; \ No newline at end of file diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index d227b06703..56136bc117 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -14,13 +14,16 @@ #include "postgres.h" +#include "access/multixact.h" #include "access/slru.h" +#include "access/xact.h" #include "access/transam.h" #include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "storage/shmem.h" #include "utils/builtins.h" +#include "utils/injection_point.h" PG_MODULE_MAGIC; @@ -36,6 +39,8 @@ PG_FUNCTION_INFO_V1(test_slru_page_sync); PG_FUNCTION_INFO_V1(test_slru_page_delete); PG_FUNCTION_INFO_V1(test_slru_page_truncate); PG_FUNCTION_INFO_V1(test_slru_delete_all); +PG_FUNCTION_INFO_V1(test_create_multixact); +PG_FUNCTION_INFO_V1(test_read_multixact); /* Number of SLRU page slots */ #define NUM_TEST_BUFFERS 16 @@ -260,3 +265,36 @@ _PG_init(void) prev_shmem_startup_hook = shmem_startup_hook; shmem_startup_hook = test_slru_shmem_startup; } + +/* + * Produces multixact with 2 current xids + */ +Datum +test_create_multixact(PG_FUNCTION_ARGS) +{ + MultiXactId id; + MultiXactIdSetOldestMember(); + id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate, + GetCurrentTransactionId(), MultiXactStatusForShare); + PG_RETURN_TRANSACTIONID(id); +} + +/* + * Reads given multixact after running an injection point. Discards local cache + * to make real read. + * This function is expected to be used in conjunction with test_create_multixact + * to test CV sleep when reading recent multixact. + */ +Datum +test_read_multixact(PG_FUNCTION_ARGS) +{ + MultiXactId id = PG_GETARG_TRANSACTIONID(0); + MultiXactMember *members; + INJECTION_POINT("test-read-multixact"); + /* discard caches */ + AtEOXact_MultiXact(); + + if (GetMultiXactIdMembers(id,&members,false, false) == -1) + elog(ERROR, "MultiXactId not found"); + PG_RETURN_VOID(); +} -- 2.39.3 (Apple Git-146)