public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey M. Borodin <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Andrew Borodin <[email protected]>
Cc: [email protected]
Cc: Yura Sokolov <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Gilles Darold <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: Anastasia Lubennikova <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: pgsql-hackers mailing list <[email protected]>
Subject: Re: MultiXact\SLRU buffers configuration
Date: Sun, 28 Jan 2024 23:17:16 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
> On 28 Jan 2024, at 17:49, Alvaro Herrera <[email protected]> wrote:
>
> I'd appreciate it if you or Horiguchi-san can update his patch to remove
> use of usleep in favor of a CV in multixact, and keep this CF entry to
> cover it.
Sure! Sounds great!
> Perhaps a test to make the code reach the usleep(1000) can be written
> using injection points (49cd2b93d7db)?
I've tried to prototype something like that. But interesting point between GetNewMultiXactId() and RecordNewMultiXact() is a critical section, and we cannot have injection points in critical sections...
Also, to implement such a test we need "wait" type of injection points, see step 2 in attachment. With this type of injection points I can stop a backend amidst entering information about new MultiXact.
Best regards, Andrey Borodin.
Attachments:
[application/octet-stream] 0001-Add-conditional-variable-to-wait-for-next-MultXact-o.patch (3.3K, ../[email protected]/2-0001-Add-conditional-variable-to-wait-for-next-MultXact-o.patch)
download | inline diff:
From 975eb3448acbec97c14d48f21261e44aca7b3acc Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Fri, 22 May 2020 14:13:33 +0500
Subject: [PATCH 1/3] Add conditional variable to wait for next MultXact offset
in edge case
---
src/backend/access/transam/multixact.c | 23 ++++++++++++++++++-
.../utils/activity/wait_event_names.txt | 1 +
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 59523be901..03fcd25d4c 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -82,6 +82,7 @@
#include "lib/ilist.h"
#include "miscadmin.h"
#include "pg_trace.h"
+#include "pgstat.h"
#include "postmaster/autovacuum.h"
#include "storage/lmgr.h"
#include "storage/pmsignal.h"
@@ -233,6 +234,7 @@ typedef struct MultiXactStateData
/* support for members anti-wraparound measures */
MultiXactOffset offsetStopLimit; /* known if oldestOffsetKnown */
+ ConditionVariable nextoff_cv;
/*
* Per-backend data starts here. We have two arrays stored in the area
* immediately following the MultiXactStateData struct. Each is indexed by
@@ -894,6 +896,14 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
/* Exchange our lock */
LWLockRelease(MultiXactOffsetSLRULock);
+ /*
+ * Let everybody know the offset of this mxid is recorded now. The waiters
+ * are waiting for the offset of the mxid next of the target to know the
+ * number of members of the target mxid, so we don't need to wait for
+ * members of this mxid are recorded.
+ */
+ ConditionVariableBroadcast(&MultiXactState->nextoff_cv);
+
LWLockAcquire(MultiXactMemberSLRULock, LW_EXCLUSIVE);
prev_pageno = -1;
@@ -1388,9 +1398,19 @@ retry:
if (nextMXOffset == 0)
{
/* Corner case 2: next multixact is still being filled in */
+
+ /*
+ * The recorder of the next mxid is just before writing the offset.
+ * Wait for the offset to be written.
+ */
+ ConditionVariablePrepareToSleep(&MultiXactState->nextoff_cv);
+
LWLockRelease(MultiXactOffsetSLRULock);
CHECK_FOR_INTERRUPTS();
- pg_usleep(1000L);
+
+ ConditionVariableSleep(&MultiXactState->nextoff_cv,
+ WAIT_EVENT_NEXT_MXMEMBERS);
+ ConditionVariableCancelSleep();
goto retry;
}
@@ -1875,6 +1895,7 @@ MultiXactShmemInit(void)
/* Make sure we zero out the per-backend state */
MemSet(MultiXactState, 0, SHARED_MULTIXACT_STATE_SIZE);
+ ConditionVariableInit(&MultiXactState->nextoff_cv);
}
else
Assert(found);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index a5df835dd4..1bb78d5aad 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -144,6 +144,7 @@ SYNC_REP "Waiting for confirmation from a remote server during synchronous repli
WAL_RECEIVER_EXIT "Waiting for the WAL receiver to exit."
WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for streaming replication."
WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated."
+NEXT_MXMEMBERS "Waiting for a next multixact member to be filled."
XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation."
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] 0002-Add-wait-type-for-injection-points.patch (3.7K, ../[email protected]/3-0002-Add-wait-type-for-injection-points.patch)
download | inline diff:
From 93b4c7dac290d6d2d212522b1a10fef03372315e Mon Sep 17 00:00:00 2001
From: "Andrey M. Borodin" <[email protected]>
Date: Sun, 28 Jan 2024 22:22:22 +0500
Subject: [PATCH 2/3] Add wait type for injection points
---
src/backend/utils/misc/injection_point.c | 20 +++++++++++++++++++
src/include/utils/injection_point.h | 1 +
src/test/modules/injection_points/Makefile | 1 +
.../injection_points/injection_points.c | 16 +++++++++++++++
4 files changed, 38 insertions(+)
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 0cf4d51cac..398ef2cf30 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -252,6 +252,26 @@ InjectionPointDetach(const char *name)
#endif
}
+/*
+ * Test if injection point is attached.
+ */
+bool
+InjectionPointIsAttach(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+ bool found;
+
+ LWLockAcquire(InjectionPointLock, LW_EXCLUSIVE);
+ hash_search(InjectionPointHash, name, HASH_FIND, &found);
+ LWLockRelease(InjectionPointLock);
+
+ return found;
+
+#else
+ elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
/*
* Execute an injection point, if defined.
*
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index 55524b568f..e07f6b7024 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -33,5 +33,6 @@ extern void InjectionPointAttach(const char *name,
const char *function);
extern void InjectionPointRun(const char *name);
extern void InjectionPointDetach(const char *name);
+extern bool InjectionPointIsAttach(const char *name);
#endif /* INJECTION_POINT_H */
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2cbbae4e0a..543d2ab927 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -7,6 +7,7 @@ DATA = injection_points--1.0.sql
PGFILEDESC = "injection_points - facility for injection points"
REGRESS = injection_points
+TAP_TESTS = 1
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..fbb30b15ad 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,6 +18,7 @@
#include "postgres.h"
#include "fmgr.h"
+#include "miscadmin.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
#include "utils/builtins.h"
@@ -28,6 +29,7 @@ PG_MODULE_MAGIC;
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
/* Set of callbacks available to be attached to an injection point. */
@@ -43,6 +45,18 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+void
+injection_wait(const char *name)
+{
+ elog(NOTICE, "waiting triggered for injection point %s", name);
+ do
+ {
+ CHECK_FOR_INTERRUPTS();
+ pg_usleep(1000L);
+ } while (InjectionPointIsAttach(name));
+ elog(NOTICE, "waiting done for injection point %s", name);
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +72,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] 0003-Try-to-test-multixact-CV-sleep.patch (5.3K, ../[email protected]/4-0003-Try-to-test-multixact-CV-sleep.patch)
download | inline diff:
From a61958407770b6cb864e0b245a4341e033824f46 Mon Sep 17 00:00:00 2001
From: "Andrey M. Borodin" <[email protected]>
Date: Sun, 28 Jan 2024 23:10:55 +0500
Subject: [PATCH 3/3] Try to test multixact CV sleep
---
src/backend/access/transam/multixact.c | 5 ++
.../injection_points--1.0.sql | 11 ++++
.../injection_points/injection_points.c | 26 ++++++++
.../modules/injection_points/t/001_wait.pl | 64 +++++++++++++++++++
4 files changed, 106 insertions(+)
create mode 100644 src/test/modules/injection_points/t/001_wait.pl
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 03fcd25d4c..15b2a0010a 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -89,6 +89,7 @@
#include "storage/proc.h"
#include "storage/procarray.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/snapmgr.h"
@@ -1200,6 +1201,8 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
LWLockRelease(MultiXactGenLock);
+ INJECTION_POINT("GetNewMultiXactId-done");
+
debug_elog4(DEBUG2, "GetNew: returning %u offset %u", result, *offset);
return result;
}
@@ -1408,6 +1411,8 @@ retry:
LWLockRelease(MultiXactOffsetSLRULock);
CHECK_FOR_INTERRUPTS();
+ INJECTION_POINT("GetMultiXactIdMembers-CV-sleep");
+
ConditionVariableSleep(&MultiXactState->nextoff_cv,
WAIT_EVENT_NEXT_MXMEMBERS);
ConditionVariableCancelSleep();
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..d3ebda5964 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -33,3 +33,14 @@ CREATE FUNCTION injection_points_detach(IN point_name TEXT)
RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_detach'
LANGUAGE C STRICT PARALLEL UNSAFE;
+
+
+CREATE FUNCTION create_test_multixact()
+RETURNS xid
+AS 'MODULE_PATHNAME', 'create_test_multixact'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+CREATE FUNCTION read_test_multixact(xid)
+RETURNS void
+AS 'MODULE_PATHNAME', 'read_test_multixact'
+LANGUAGE C STRICT PARALLEL UNSAFE;
\ No newline at end of file
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index fbb30b15ad..d3d5faaa25 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -109,3 +109,29 @@ injection_points_detach(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+#include "access/multixact.h"
+#include "access/xact.h"
+
+PG_FUNCTION_INFO_V1(create_test_multixact);
+Datum
+create_test_multixact(PG_FUNCTION_ARGS)
+{
+ MultiXactId id;
+ MultiXactIdSetOldestMember();
+ id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate,
+ GetCurrentTransactionId(), MultiXactStatusForShare);
+ PG_RETURN_TRANSACTIONID(id);
+}
+
+PG_FUNCTION_INFO_V1(read_test_multixact);
+Datum
+read_test_multixact(PG_FUNCTION_ARGS)
+{
+ MultiXactId id = PG_GETARG_TRANSACTIONID(0);
+ MultiXactMember *members;
+ INJECTION_POINT("read_test_multixact");
+ if (GetMultiXactIdMembers(id,&members,false, false) == -1)
+ elog(ERROR, "MultiXactId not found");
+ PG_RETURN_VOID();
+}
\ No newline at end of file
diff --git a/src/test/modules/injection_points/t/001_wait.pl b/src/test/modules/injection_points/t/001_wait.pl
new file mode 100644
index 0000000000..6076ffc6ea
--- /dev/null
+++ b/src/test/modules/injection_points/t/001_wait.pl
@@ -0,0 +1,64 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my ($node, $result);
+
+$node = PostgreSQL::Test::Cluster->new('injection_points');
+$node->init;
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION injection_points));
+
+$result = $node->psql('postgres', q(select injection_points_attach('FIRST','wait')));
+is($result, '0', 'wait injection point set');
+
+my $bg = $node->background_psql('postgres');
+
+$bg->query_until(
+ qr/start/, q(
+\echo start
+select injection_points_run('FIRST');
+select injection_points_attach('SECOND','wait');
+));
+
+$result = $node->psql('postgres', q(
+select injection_points_run('SECOND');
+select injection_points_detach('FIRST');
+));
+is($result, '0', 'wait injection point set');
+
+$bg->quit;
+
+$node->safe_psql('postgres', q(select injection_points_attach('read_test_multixact','wait')));
+$node->safe_psql('postgres', q(select injection_points_attach('GetMultiXactIdMembers-CV-sleep','notice')));
+
+my $observer = $node->background_psql('postgres');
+
+$observer->query_safe(
+ q(
+select read_test_multixact(create_test_multixact());
+));
+
+$node->safe_psql('postgres', q(select injection_points_attach('GetNewMultiXactId-done','wait')));
+
+my $creator = $node->background_psql('postgres');
+
+$creator->query_safe( q(select create_test_multixact();));
+
+$node->safe_psql('postgres', q(select injection_points_detach('read_test_multixact')));
+
+$node->safe_psql('postgres', q(select injection_points_detach('GetNewMultiXactId-done')));
+
+$observer->quit;
+
+$creator->quit;
+
+$node->stop;
+done_testing();
--
2.37.1 (Apple Git-137.1)
view thread (88+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: MultiXact\SLRU buffers configuration
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox