From 121f2da707ad4a71f0b3a6f70002190bae066cc5 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Mon, 20 May 2024 15:20:31 +0500 Subject: [PATCH vAB1 2/2] Add multixact CV sleep test Previosuly we used 1ms sleep when we are reading multixact before another multixact withouth offset yet. a0e0fb1ba changed this behaviour to CV sleep. This commit addes test case for this. To test such race condition we have to use waiting injection point under critical section. This requires special pre-loaded injection point. --- 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 54c916e034..8c68bdbe0b 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" @@ -825,8 +826,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("GetNewMultiXactId-done"); + multi = GetNewMultiXactId(nmembers, &offset); + INJECTION_POINT("GetNewMultiXactId-done"); + /* Make an XLOG entry describing the new MXID. */ xlrec.mid = multi; xlrec.moff = offset; @@ -1439,6 +1444,8 @@ retry: LWLockRelease(lock); CHECK_FOR_INTERRUPTS(); + INJECTION_POINT("GetMultiXactIdMembers-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..5c24bf2736 --- /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 offest 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('GetMultiXactIdMembers-CV-sleep','wait'))); + +# This session must observe sleep on CV when generating multixact. +# To achive 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('GetNewMultiXactId-done','wait');)); + +# We expect this query to hand in critical section after generating new multixact, +# but before filling it's offset into SLRU +$creator->query_until(qr/start/, q( + \echo start + select injection_points_load('GetNewMultiXactId-done'); + select test_create_multixact(); +)); + +$node->wait_for_event('client backend', 'GetNewMultiXactId-done'); + +# 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', 'GetMultiXactIdMembers-CV-sleep'); + +# Now we have two backends waiting in GetNewMultiXactId-done and +# GetMultiXactIdMembers-CV-sleep. Also we have 3 injections points set to wait. +# If we wakeup GetMultiXactIdMembers-CV-sleep it will happend 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('GetNewMultiXactId-done'))); +$node->safe_psql('postgres', q(select injection_points_detach('GetMultiXactIdMembers-CV-sleep'))); + +$node->safe_psql('postgres', q(select injection_points_wakeup('GetNewMultiXactId-done'))); +$node->safe_psql('postgres', q(select injection_points_wakeup('GetMultiXactIdMembers-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..699a9cd64b 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)