public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 8/8] Ignore correlation for new BRIN opclasses
4+ messages / 3 participants
[nested] [flat]
* [PATCH 8/8] Ignore correlation for new BRIN opclasses
@ 2020-09-12 13:07 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw)
The new BRIN opclasses (bloom and minmax-multi) are less sensitive to
poorly correlated data, so just assume the data is perfectly correlated
during costing.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin_bloom.c | 1 +
src/backend/access/brin/brin_minmax_multi.c | 1 +
src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++-
src/include/access/brin_internal.h | 3 +++
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index 4494484b3b..46ffc068be 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -412,6 +412,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(BloomOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (BloomOpaque *)
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index c90c721cac..a6f79311e2 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -1737,6 +1737,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(MinmaxMultiOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (MinmaxMultiOpaque *)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 52314d3aa1..0320d128f6 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -98,6 +98,7 @@
#include <math.h>
#include "access/brin.h"
+#include "access/brin_internal.h"
#include "access/brin_page.h"
#include "access/gin.h"
#include "access/table.h"
@@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
double minimalRanges;
double estimatedRanges;
double selec;
- Relation indexRel;
+ Relation indexRel = NULL;
+ TupleDesc tupdesc = NULL;
ListCell *l;
VariableStatData vardata;
@@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
*/
indexRel = index_open(index->indexoid, NoLock);
brinGetStats(indexRel, &statsData);
+ tupdesc = RelationGetDescr(indexRel);
index_close(indexRel, NoLock);
/* work out the actual number of ranges in the index */
@@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
{
IndexClause *iclause = lfirst_node(IndexClause, l);
AttrNumber attnum = index->indexkeys[iclause->indexcol];
+ FmgrInfo *opcInfoFn;
+ BrinOpcInfo *opcInfo;
+ Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol);
+ bool ignore_correlation;
+
+ opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO);
+
+ opcInfo = (BrinOpcInfo *)
+ DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid));
+
+ ignore_correlation = opcInfo->oi_ignore_correlation;
/* attempt to lookup stats in relation for this index column */
if (attnum != 0)
@@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
if (sslot.nnumbers > 0)
varCorrelation = Abs(sslot.numbers[0]);
+ if (ignore_correlation)
+ varCorrelation = 1.0;
+
if (varCorrelation > *indexCorrelation)
*indexCorrelation = varCorrelation;
diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index fdaff42722..5fbf8cf9c7 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -30,6 +30,9 @@ typedef struct BrinOpcInfo
/* Regular processing of NULLs in BrinValues? */
bool oi_regular_nulls;
+ /* Ignore correlation during cost estimation */
+ bool oi_ignore_correlation;
+
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
--
2.26.2
--------------AC3FFB734F56C5F52D422EFC--
^ permalink raw reply [nested|flat] 4+ messages in thread
* Adding facility for injection points (or probe points?) for more advanced tests
@ 2023-10-25 04:13 Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2023-10-25 04:13 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
Hi all,
I don't remember how many times in the last few years when I've had to
hack the backend to produce a test case that involves a weird race
condition across multiple processes running in the backend, to be able
to prove a point or just test a fix (one recent case: 2b8e5273e949).
Usually, I come to hardcoding stuff for the following situations:
- Trigger a PANIC, to force recovery.
- A FATAL, to take down a session, or just an ERROR.
- palloc() failure injection.
- Sleep to slow down a code path.
- Pause and release with condition variable.
And, while that's helpful to prove a point on a thread, nothing comes
out of it in terms of regression test coverage in the tree because
these tests are usually too slow and expensive, as they usually rely
on hardcoded timeouts. So that's pretty much attempting to emulate
what one would do with a debugger in a predictable way, without the
manual steps because human hands don't scale well.
The reason behind that is of course more advanced testing, to be able
to expand coverage when we have weird and non-deterministic race
issues to deal with, and the code becoming more complex every year
makes that even harder. Fault and failure injection in specific paths
comes into mind, additionally, particularly if you manage complex
projects based on Postgres.
So, please find attached a patch set that introduces an in-core
facility to be able to set what I'm calling here an "injection point",
that consists of being able to register in shared memory a callback
that can be run within a defined location of the code. It means that
it is not possible to trigger a callback before shared memory is set,
but I've faced far more the case where I wanted to trigger something
after shmem is set anyway. Persisting an injection point across
restarts is also possible by adding some through an extension's shmem
hook, as we do for custom LWLocks for example, as long as a library is
loaded.
This will remind a bit of what Alexander Korotkov has proposed here:
https://www.postgresql.org/message-id/CAPpHfdtSEOHX8dSk9Qp%2BZ%2B%2Bi4BGQoffKip6JDWngEA%2Bg7Z-XmQ%40...
Also, this is much closee to what Craig Ringer is mentioning here,
where it is named probe points, but I am using a minimal design that
allows to achieve the same:
https://www.postgresql.org/message-id/CAPpHfdsn-hzneYNbX4qcY5rnwr-BA1ogOCZ4TQCKQAw9qa48kA%40mail.gma...
A difference is that I don't really see a point in passing to the
callback triggered an area of data coming from the hash table itself,
as at the end a callback could just refer to an area in shared memory
or a static set of variables depending on what it wants, with one or
more injection points (say a location to set a state, and a second to
check it). So, at the end, the problem comes down in my opinion to
two things:
- Possibility to trigger a condition defined by some custom code, in
the backend (core code or even out-of-core).
- Possibility to define a location in the code where a named point
would be checked.
0001 introduces three APIs to create, run, and drop injection points:
+extern void InjectionPointCreate(const char *name,
+ InjectionPointCallback callback);
+extern void InjectionPointRun(const char *name);
+extern void InjectionPointDrop(const char *name);
Then one just needs to add a macro like that to trigger the callback
registered in the code to test:
INJECTION_POINT_RUN("String");
So the footprint in the core tree is not zero, but it is as minimal as
it can be.
I have added some documentation to explain that, as well. I am not
wedded to the name proposed in the patch, so if you feel there is
better, feel free to propose ideas.
This facility is hidden behind a specific configure/Meson switch,
making it a no-op by default:
--enable-injection-points
-Dinjection_points={ true | false }
0002 is a test module to test these routines, that I have kept a
maximum simple to ease review of the basics proposed here. This could
be extended further to propose more default modes with TAP tests on
its own, as I don't see a real point in having the SQL bits or some
common callbacks (like for the PANIC or the FATAL cases) in core.
Thoughts and comments are welcome.
--
Michael
Attachments:
[text/x-diff] 0001-Base-facility-for-injection-points.patch (17.2K, ../../[email protected]/2-0001-Base-facility-for-injection-points.patch)
download | inline diff:
From 4f7a0627eb0735874b53c90576243f8003a64b4a Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Wed, 25 Oct 2023 10:24:28 +0900
Subject: [PATCH 1/2] Base facility for injection points
Extension and backend developers can register custom callbacks that
would be run on a user-defined fashion. This is hidden behind a
./configure and a meson switch, disabled by default.
---
src/include/pg_config.h.in | 3 +
src/include/utils/injection_point.h | 35 ++++
src/backend/storage/ipc/ipci.c | 3 +
src/backend/storage/lmgr/lwlocknames.txt | 1 +
.../utils/activity/wait_event_names.txt | 1 +
src/backend/utils/misc/Makefile | 1 +
src/backend/utils/misc/injection_point.c | 158 ++++++++++++++++++
src/backend/utils/misc/meson.build | 1 +
doc/src/sgml/installation.sgml | 30 ++++
doc/src/sgml/xfunc.sgml | 53 ++++++
configure | 32 ++++
configure.ac | 6 +
meson.build | 1 +
meson_options.txt | 3 +
src/tools/pgindent/typedefs.list | 1 +
15 files changed, 329 insertions(+)
create mode 100644 src/include/utils/injection_point.h
create mode 100644 src/backend/utils/misc/injection_point.c
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index d8a2985567f..7a976821e55 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -701,6 +701,9 @@
/* Define to build with ICU support. (--with-icu) */
#undef USE_ICU
+/* Define to 1 to build with injection points. (--enable-injection-points) */
+#undef USE_INJECTION_POINTS
+
/* Define to 1 to build with LDAP support. (--with-ldap) */
#undef USE_LDAP
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
new file mode 100644
index 00000000000..3edc80fb8fe
--- /dev/null
+++ b/src/include/utils/injection_point.h
@@ -0,0 +1,35 @@
+/*-------------------------------------------------------------------------
+ * injection_point.h
+ * Definitions related to injection points.
+ *
+ * Copyright (c) 2001-2023, PostgreSQL Global Development Group
+ *
+ * src/include/utils/injection_point.h
+ * ----------
+ */
+#ifndef INJECTION_POINT_H
+#define INJECTION_POINT_H
+
+/*
+ * Typedef for callback function launched by an injection point.
+ */
+typedef void (*InjectionPointCallback) (const char *name);
+
+/*
+ * Injections points require --enable-injection-points.
+ */
+#ifdef USE_INJECTION_POINTS
+#define INJECTION_POINT_RUN(name) InjectionPointRun(name)
+#else
+#define INJECTION_POINT_RUN(name)
+#endif
+
+extern Size InjectionPointShmemSize(void);
+extern void InjectionPointShmemInit(void);
+
+extern void InjectionPointCreate(const char *name,
+ InjectionPointCallback callback);
+extern void InjectionPointRun(const char *name);
+extern void InjectionPointDrop(const char *name);
+
+#endif /* INJECTION_POINT_H */
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index a3d8eacb8dc..12b8a42fd9b 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -48,6 +48,7 @@
#include "storage/sinvaladt.h"
#include "storage/spin.h"
#include "utils/guc.h"
+#include "utils/injection_point.h"
#include "utils/snapmgr.h"
#include "utils/wait_event.h"
@@ -143,6 +144,7 @@ CalculateShmemSize(int *num_semaphores)
size = add_size(size, AsyncShmemSize());
size = add_size(size, StatsShmemSize());
size = add_size(size, WaitEventExtensionShmemSize());
+ size = add_size(size, InjectionPointShmemSize());
#ifdef EXEC_BACKEND
size = add_size(size, ShmemBackendArraySize());
#endif
@@ -302,6 +304,7 @@ CreateSharedMemoryAndSemaphores(void)
AsyncShmemInit();
StatsShmemInit();
WaitEventExtensionShmemInit();
+ InjectionPointShmemInit();
#ifdef EXEC_BACKEND
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt
index f72f2906cea..42a048746df 100644
--- a/src/backend/storage/lmgr/lwlocknames.txt
+++ b/src/backend/storage/lmgr/lwlocknames.txt
@@ -54,3 +54,4 @@ XactTruncationLock 44
WrapLimitsVacuumLock 46
NotifyQueueTailLock 47
WaitEventExtensionLock 48
+InjectionPointLock 49
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index d7995931bd4..5631d291383 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -319,6 +319,7 @@ XactTruncation "Waiting to execute <function>pg_xact_status</function> or update
WrapLimitsVacuum "Waiting to update limits on transaction id and multixact consumption."
NotifyQueueTail "Waiting to update limit on <command>NOTIFY</command> message storage."
WaitEventExtension "Waiting to read or update custom wait events information for extensions."
+InjectionPoint "Waiting to read or update information related to injection points."
XactBuffer "Waiting for I/O on a transaction status SLRU buffer."
CommitTsBuffer "Waiting for I/O on a commit timestamp SLRU buffer."
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index 29100329300..f0203501d12 100644
--- a/src/backend/utils/misc/Makefile
+++ b/src/backend/utils/misc/Makefile
@@ -21,6 +21,7 @@ OBJS = \
guc_funcs.o \
guc_tables.o \
help_config.o \
+ injection_point.o \
pg_config.o \
pg_controldata.o \
pg_rusage.o \
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
new file mode 100644
index 00000000000..b9701fbb3a6
--- /dev/null
+++ b/src/backend/utils/misc/injection_point.c
@@ -0,0 +1,158 @@
+/*-------------------------------------------------------------------------
+ *
+ * injection_point.c
+ * Routines to control and run injection points in the code.
+ *
+ * Injection points can be used to call arbitrary callbacks in specific
+ * places of the code, registering callbacks that would be run in the code
+ * paths where a named injection point exists.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/utils/misc/injection_point.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "miscadmin.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+#include "utils/hsearch.h"
+#include "utils/injection_point.h"
+
+/*
+ * Hash table for storing injection points.
+ *
+ * InjectionPointHashByName is used to find an injection point by name.
+ */
+static HTAB *InjectionPointHashByName; /* find points from names */
+
+/* Field sizes */
+#define INJ_MAXLEN 64
+
+typedef struct InjectionPointEntryByName
+{
+ char name[INJ_MAXLEN]; /* hash key */
+ InjectionPointCallback callback;
+} InjectionPointEntryByName;
+
+#define INJECTION_POINT_HASH_INIT_SIZE 16
+#define INJECTION_POINT_HASH_MAX_SIZE 128
+
+/*
+ * Return the space for dynamic shared hash table.
+ */
+Size
+InjectionPointShmemSize(void)
+{
+#ifdef USE_INJECTION_POINTS
+ Size sz = 0;
+
+ sz = add_size(sz, hash_estimate_size(INJECTION_POINT_HASH_MAX_SIZE,
+ sizeof(InjectionPointEntryByName)));
+ return sz;
+#endif
+}
+
+/*
+ * Allocate shmem space for dynamic shared hash.
+ */
+void
+InjectionPointShmemInit(void)
+{
+#ifdef USE_INJECTION_POINTS
+ HASHCTL info;
+
+ /* key is a NULL-terminated string */
+ info.keysize = sizeof(char[INJ_MAXLEN]);
+ info.entrysize = sizeof(InjectionPointEntryByName);
+ InjectionPointHashByName = ShmemInitHash("InjectionPoint hash by name",
+ INJECTION_POINT_HASH_INIT_SIZE,
+ INJECTION_POINT_HASH_MAX_SIZE,
+ &info,
+ HASH_ELEM | HASH_STRINGS);
+#endif
+}
+
+/*
+ * Insert a new injection point.
+ */
+void
+InjectionPointCreate(const char *name, InjectionPointCallback callback)
+{
+#ifdef USE_INJECTION_POINTS
+ InjectionPointEntryByName *entry_by_name;
+ bool found;
+
+ if (strlen(name) >= INJ_MAXLEN)
+ elog(ERROR, "injection point name %s too long", name);
+
+ /*
+ * Allocate and register a new injection point. A new point should not
+ * exist. For testing purposes this should be fine.
+ */
+ LWLockAcquire(InjectionPointLock, LW_EXCLUSIVE);
+ entry_by_name = (InjectionPointEntryByName *)
+ hash_search(InjectionPointHashByName, name,
+ HASH_ENTER, &found);
+ if (found)
+ {
+ LWLockRelease(InjectionPointLock);
+ elog(ERROR, "injection point \"%s\" already defined", name);
+ }
+
+ /* Save the entry */
+ memcpy(entry_by_name->name, name, sizeof(entry_by_name->name));
+ entry_by_name->name[INJ_MAXLEN - 1] = '\0';
+ entry_by_name->callback = callback;
+
+ LWLockRelease(InjectionPointLock);
+#else
+ elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
+void
+InjectionPointDrop(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+ bool found;
+
+ LWLockAcquire(InjectionPointLock, LW_EXCLUSIVE);
+ hash_search(InjectionPointHashByName, name, HASH_REMOVE, &found);
+ LWLockRelease(InjectionPointLock);
+
+ if (!found)
+ elog(ERROR, "injection point \"%s\" not found", name);
+#else
+ elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
+void
+InjectionPointRun(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+ InjectionPointEntryByName *entry_by_name;
+ bool found;
+
+ LWLockAcquire(InjectionPointLock, LW_SHARED);
+ entry_by_name = (InjectionPointEntryByName *)
+ hash_search(InjectionPointHashByName, name,
+ HASH_FIND, &found);
+ LWLockRelease(InjectionPointLock);
+
+ /* If not found, do nothing? */
+ if (!found)
+ return;
+
+ /* Found, so just run the callback registered */
+ entry_by_name->callback(entry_by_name->name);
+#else
+ elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
diff --git a/src/backend/utils/misc/meson.build b/src/backend/utils/misc/meson.build
index f719c97c051..1438859b695 100644
--- a/src/backend/utils/misc/meson.build
+++ b/src/backend/utils/misc/meson.build
@@ -6,6 +6,7 @@ backend_sources += files(
'guc_funcs.c',
'guc_tables.c',
'help_config.c',
+ 'injection_point.c',
'pg_config.c',
'pg_controldata.c',
'pg_rusage.c',
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 9b25e9fdb1b..ba055ad4a54 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1677,6 +1677,21 @@ build-postgresql:
</listitem>
</varlistentry>
+ <varlistentry id="configure-option-enable-injection-points">
+ <term><option>--enable-injection-points</option></term>
+ <listitem>
+ <para>
+ Compiles <productname>PostgreSQL</productname> with support for
+ injection points in the server. This is valuable to inject
+ user-defined code to force specific conditions to happen on the
+ server in pre-defined code paths. This option is disabled by default.
+ See <xref linkend="xfunc-addin-injection-points"/> for more details.
+ This option is only for developers to test specific concurrency
+ scenarios.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="configure-option-with-segsize-blocks">
<term><option>--with-segsize-blocks=SEGSIZE_BLOCKS</option></term>
<listitem>
@@ -3174,6 +3189,21 @@ ninja install
</listitem>
</varlistentry>
+ <varlistentry id="configure-injection-points-meson">
+ <term><option>-Dinjection_points={ true | false }</option></term>
+ <listitem>
+ <para>
+ Compiles <productname>PostgreSQL</productname> with support for
+ injection points in the server. This is valuable to inject
+ user-defined code to force specific conditions to happen on the
+ server in pre-defined code paths. This option is disabled by default.
+ See <xref linkend="xfunc-addin-injection-points"/> for more details.
+ This option is only for developers to test specific concurrency
+ scenarios.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="configure-segsize-blocks-meson">
<term><option>-Dsegsize_blocks=SEGSIZE_BLOCKS</option></term>
<listitem>
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 96ba95818c2..e8dd262152e 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3480,6 +3480,59 @@ uint32 WaitEventExtensionNew(const char *wait_event_name)
</para>
</sect2>
+ <sect2 id="xfunc-addin-injection-points">
+ <title>Injection Points</title>
+
+ <para>
+ Add-ins can define injection points, that can register callbacks
+ to run user-defined code when going through a specific code path,
+ by calling:
+<programlisting>
+extern void InjectionPointCreate(const char *name,
+ InjectionPointCallback callback);
+</programlisting>
+
+ <literal>name</literal> is the name of the injection point.
+ Injection points are saved in a hash table in shared memory, and
+ last until the server is shut down.
+ </para>
+
+ <para>
+ Here is an example of callback for
+ <literal>InjectionPointCallback</literal>:
+<programlisting>
+static void
+custom_injection_callback(const char *name)
+{
+ elog(NOTICE, "%s: executed custom callback", name);
+}
+</programlisting>
+ </para>
+
+ <para>
+ Once an injection point is defined, running it requires to use
+ the following macro to trigger the callback given in a wanted code
+ path:
+<programlisting>
+INJECTION_POINT_RUN(name);
+</programlisting>
+ </para>
+
+ <para>
+ Optionally, it is possible to drop injection points by calling:
+<programlisting>
+extern void InjectionPointDrop(const char *name);
+</programlisting>
+ </para>
+
+ <para>
+ Enabling injections points requires
+ <option>--enable-injection-points</option> from
+ <command>configure</command> or <option>-Dinjection_points=true</option>
+ from <application>Meson</application>.
+ </para>
+ </sect2>
+
<sect2 id="extend-cpp">
<title>Using C++ for Extensibility</title>
diff --git a/configure b/configure
index cfd968235f7..25ed062ce1f 100755
--- a/configure
+++ b/configure
@@ -839,6 +839,7 @@ enable_profiling
enable_coverage
enable_dtrace
enable_tap_tests
+enable_injection_points
with_blocksize
with_segsize
with_segsize_blocks
@@ -1532,6 +1533,8 @@ Optional Features:
--enable-coverage build with coverage testing instrumentation
--enable-dtrace build with DTrace support
--enable-tap-tests enable TAP tests (requires Perl and IPC::Run)
+ --enable-injection-points
+ enable injection points (for testing)
--enable-depend turn on automatic dependency tracking
--enable-cassert enable assertion checks (for debugging)
--disable-largefile omit support for large files
@@ -3682,6 +3685,35 @@ fi
+#
+# Injection points
+#
+
+
+# Check whether --enable-injection-points was given.
+if test "${enable_injection_points+set}" = set; then :
+ enableval=$enable_injection_points;
+ case $enableval in
+ yes)
+
+$as_echo "#define USE_INJECTION_POINTS 1" >>confdefs.h
+
+ ;;
+ no)
+ :
+ ;;
+ *)
+ as_fn_error $? "no argument expected for --enable-injection-points option" "$LINENO" 5
+ ;;
+ esac
+
+else
+ enable_injection_points=no
+
+fi
+
+
+
#
# Block size
#
diff --git a/configure.ac b/configure.ac
index f220b379b30..d9c734cf135 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,12 @@ PGAC_ARG_BOOL(enable, tap-tests, no,
[enable TAP tests (requires Perl and IPC::Run)])
AC_SUBST(enable_tap_tests)
+#
+# Injection points
+#
+PGAC_ARG_BOOL(enable, injection-points, no, [enable injection points (for testing)],
+ [AC_DEFINE([USE_INJECTION_POINTS], 1, [Define to 1 to build with injection points. (--enable-injection-points)])])
+
#
# Block size
#
diff --git a/meson.build b/meson.build
index 2d516c8f372..232bc87e30b 100644
--- a/meson.build
+++ b/meson.build
@@ -431,6 +431,7 @@ meson_bin = find_program(meson_binpath, native: true)
###############################################################
cdata.set('USE_ASSERT_CHECKING', get_option('cassert') ? 1 : false)
+cdata.set('USE_INJECTION_POINTS', get_option('injection_points') ? 1 : false)
blocksize = get_option('blocksize').to_int() * 1024
diff --git a/meson_options.txt b/meson_options.txt
index d2f95cfec36..6041ab65410 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -43,6 +43,9 @@ option('cassert', type: 'boolean', value: false,
option('tap_tests', type: 'feature', value: 'auto',
description: 'Enable TAP tests')
+option('injection_points', type: 'boolean', value: true,
+ description: 'Enable injection points')
+
option('PG_TEST_EXTRA', type: 'string', value: '',
description: 'Enable selected extra tests')
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 06b25617bc9..ba7c5f3c75a 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1136,6 +1136,7 @@ IdentLine
IdentifierLookup
IdentifySystemCmd
IfStackElem
+InjectionPointEntryByName
ImportForeignSchemaStmt
ImportForeignSchemaType
ImportForeignSchema_function
--
2.42.0
[text/x-diff] 0002-Add-test-module-for-injection-points.patch (13.5K, ../../[email protected]/3-0002-Add-test-module-for-injection-points.patch)
download | inline diff:
From b7d39e5695ef1cdcdc53f6b811b947c186f7a433 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Wed, 25 Oct 2023 10:43:05 +0900
Subject: [PATCH 2/2] Add test module for injection points
---
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
.../modules/test_injection_points/.gitignore | 4 +
.../modules/test_injection_points/Makefile | 22 +++++
.../expected/test_injection_points.out | 45 ++++++++++
.../expected/test_injection_points_1.out | 32 +++++++
.../modules/test_injection_points/meson.build | 33 +++++++
.../sql/test_injection_points.sql | 15 ++++
.../test_injection_points--1.0.sql | 36 ++++++++
.../test_injection_points.c | 88 +++++++++++++++++++
.../test_injection_points.control | 4 +
doc/src/sgml/xfunc.sgml | 3 +
12 files changed, 284 insertions(+)
create mode 100644 src/test/modules/test_injection_points/.gitignore
create mode 100644 src/test/modules/test_injection_points/Makefile
create mode 100644 src/test/modules/test_injection_points/expected/test_injection_points.out
create mode 100644 src/test/modules/test_injection_points/expected/test_injection_points_1.out
create mode 100644 src/test/modules/test_injection_points/meson.build
create mode 100644 src/test/modules/test_injection_points/sql/test_injection_points.sql
create mode 100644 src/test/modules/test_injection_points/test_injection_points--1.0.sql
create mode 100644 src/test/modules/test_injection_points/test_injection_points.c
create mode 100644 src/test/modules/test_injection_points/test_injection_points.control
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index e81873cb5ae..a0ca613b1cf 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -19,6 +19,7 @@ SUBDIRS = \
test_ddl_deparse \
test_extensions \
test_ginpostinglist \
+ test_injection_points \
test_integerset \
test_lfind \
test_misc \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index fcd643f6f10..67e9eff878a 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -16,6 +16,7 @@ subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
subdir('test_extensions')
subdir('test_ginpostinglist')
+subdir('test_injection_points')
subdir('test_integerset')
subdir('test_lfind')
subdir('test_misc')
diff --git a/src/test/modules/test_injection_points/.gitignore b/src/test/modules/test_injection_points/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_injection_points/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_injection_points/Makefile b/src/test/modules/test_injection_points/Makefile
new file mode 100644
index 00000000000..65bcdde782f
--- /dev/null
+++ b/src/test/modules/test_injection_points/Makefile
@@ -0,0 +1,22 @@
+# src/test/modules/test_injection_points/Makefile
+
+MODULE_big = test_injection_points
+OBJS = \
+ $(WIN32RES) \
+ test_injection_points.o
+PGFILEDESC = "test_injection_points - test injection points"
+
+EXTENSION = test_injection_points
+DATA = test_injection_points--1.0.sql
+REGRESS = test_injection_points
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_injection_points
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_injection_points/expected/test_injection_points.out b/src/test/modules/test_injection_points/expected/test_injection_points.out
new file mode 100644
index 00000000000..c85639146cd
--- /dev/null
+++ b/src/test/modules/test_injection_points/expected/test_injection_points.out
@@ -0,0 +1,45 @@
+CREATE EXTENSION test_injection_points;
+SELECT test_injection_points_create('TestInjectionBooh', 'booh');
+ERROR: incorrect mode "booh" for injection point creation
+SELECT test_injection_points_create('TestInjectionError', 'error');
+ test_injection_points_create
+------------------------------
+
+(1 row)
+
+SELECT test_injection_points_create('TestInjectionLog', 'notice');
+ test_injection_points_create
+------------------------------
+
+(1 row)
+
+SELECT test_injection_points_run('TestInjectionBooh'); -- nothing happens
+ test_injection_points_run
+---------------------------
+
+(1 row)
+
+SELECT test_injection_points_run('TestInjectionLog');
+NOTICE: notice triggered for injection point TestInjectionLog
+ test_injection_points_run
+---------------------------
+
+(1 row)
+
+SELECT test_injection_points_run('TestInjectionError');
+ERROR: error triggered for injection point TestInjectionError
+SELECT test_injection_points_drop('TestInjectionError'); -- ok
+ test_injection_points_drop
+----------------------------
+
+(1 row)
+
+SELECT test_injection_points_drop('TestInjectionLog'); -- ok
+ test_injection_points_drop
+----------------------------
+
+(1 row)
+
+SELECT test_injection_points_drop('TestInjectionLog'); -- fails
+ERROR: injection point "TestInjectionLog" not found
+DROP EXTENSION test_injection_points;
diff --git a/src/test/modules/test_injection_points/expected/test_injection_points_1.out b/src/test/modules/test_injection_points/expected/test_injection_points_1.out
new file mode 100644
index 00000000000..68d14b2d5ca
--- /dev/null
+++ b/src/test/modules/test_injection_points/expected/test_injection_points_1.out
@@ -0,0 +1,32 @@
+CREATE EXTENSION test_injection_points;
+SELECT test_injection_points_create('TestInjectionBooh', 'booh');
+ERROR: incorrect mode "booh" for injection point creation
+SELECT test_injection_points_create('TestInjectionError', 'error');
+ERROR: Injection points are not supported by this build
+SELECT test_injection_points_create('TestInjectionLog', 'notice');
+ERROR: Injection points are not supported by this build
+SELECT test_injection_points_run('TestInjectionBooh'); -- nothing happens
+ test_injection_points_run
+---------------------------
+
+(1 row)
+
+SELECT test_injection_points_run('TestInjectionLog');
+ test_injection_points_run
+---------------------------
+
+(1 row)
+
+SELECT test_injection_points_run('TestInjectionError');
+ test_injection_points_run
+---------------------------
+
+(1 row)
+
+SELECT test_injection_points_drop('TestInjectionError'); -- ok
+ERROR: Injection points are not supported by this build
+SELECT test_injection_points_drop('TestInjectionLog'); -- ok
+ERROR: Injection points are not supported by this build
+SELECT test_injection_points_drop('TestInjectionLog'); -- fails
+ERROR: Injection points are not supported by this build
+DROP EXTENSION test_injection_points;
diff --git a/src/test/modules/test_injection_points/meson.build b/src/test/modules/test_injection_points/meson.build
new file mode 100644
index 00000000000..49e29ac8a9a
--- /dev/null
+++ b/src/test/modules/test_injection_points/meson.build
@@ -0,0 +1,33 @@
+# Copyright (c) 2022-2023, PostgreSQL Global Development Group
+
+test_injection_points_sources = files(
+ 'test_injection_points.c',
+)
+
+if host_system == 'windows'
+ test_injection_points_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+ '--NAME', 'test_injection_points',
+ '--FILEDESC', 'test_injection_points - test injection points',])
+endif
+
+test_injection_points = shared_module('test_injection_points',
+ test_injection_points_sources,
+ kwargs: pg_test_mod_args,
+)
+test_install_libs += test_injection_points
+
+test_install_data += files(
+ 'test_injection_points.control',
+ 'test_injection_points--1.0.sql',
+)
+
+tests += {
+ 'name': 'test_injection_points',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'regress': {
+ 'sql': [
+ 'test_injection_points',
+ ],
+ },
+}
diff --git a/src/test/modules/test_injection_points/sql/test_injection_points.sql b/src/test/modules/test_injection_points/sql/test_injection_points.sql
new file mode 100644
index 00000000000..44135c7c374
--- /dev/null
+++ b/src/test/modules/test_injection_points/sql/test_injection_points.sql
@@ -0,0 +1,15 @@
+CREATE EXTENSION test_injection_points;
+
+SELECT test_injection_points_create('TestInjectionBooh', 'booh');
+SELECT test_injection_points_create('TestInjectionError', 'error');
+SELECT test_injection_points_create('TestInjectionLog', 'notice');
+
+SELECT test_injection_points_run('TestInjectionBooh'); -- nothing happens
+SELECT test_injection_points_run('TestInjectionLog');
+SELECT test_injection_points_run('TestInjectionError');
+
+SELECT test_injection_points_drop('TestInjectionError'); -- ok
+SELECT test_injection_points_drop('TestInjectionLog'); -- ok
+SELECT test_injection_points_drop('TestInjectionLog'); -- fails
+
+DROP EXTENSION test_injection_points;
diff --git a/src/test/modules/test_injection_points/test_injection_points--1.0.sql b/src/test/modules/test_injection_points/test_injection_points--1.0.sql
new file mode 100644
index 00000000000..c4c42b5b699
--- /dev/null
+++ b/src/test/modules/test_injection_points/test_injection_points--1.0.sql
@@ -0,0 +1,36 @@
+/* src/test/modules/test_injection_points/test_injection_points--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_injection_points" to load this file. \quit
+
+--
+-- test_injection_points_create()
+--
+-- Creates an injection point using callbacks from one of the predefined
+-- modes.
+--
+CREATE FUNCTION test_injection_points_create(IN point_name TEXT,
+ IN mode text)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_injection_points_create'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+--
+-- test_injection_points_run()
+--
+-- Executes an injection point.
+--
+CREATE FUNCTION test_injection_points_run(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_injection_points_run'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+--
+-- test_injection_points_drop()
+--
+-- Drops an injection point.
+--
+CREATE FUNCTION test_injection_points_drop(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_injection_points_drop'
+LANGUAGE C STRICT PARALLEL UNSAFE;
diff --git a/src/test/modules/test_injection_points/test_injection_points.c b/src/test/modules/test_injection_points/test_injection_points.c
new file mode 100644
index 00000000000..a842d837e82
--- /dev/null
+++ b/src/test/modules/test_injection_points/test_injection_points.c
@@ -0,0 +1,88 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_injection_points.c
+ * Code for testing injection points.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_injection_points/test_injection_points.c
+ *
+ * Injection points are able to trigger user-defined callbacks in pre-defined
+ * code paths.
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "utils/builtins.h"
+#include "utils/injection_point.h"
+
+PG_MODULE_MAGIC;
+
+/* Set of callbacks available at point creation */
+static void
+test_injection_error(const char *name)
+{
+ elog(ERROR, "error triggered for injection point %s", name);
+}
+
+static void
+test_injection_notice(const char *name)
+{
+ elog(NOTICE, "notice triggered for injection point %s", name);
+}
+
+/*
+ * SQL function for creating an injection point.
+ */
+PG_FUNCTION_INFO_V1(test_injection_points_create);
+Datum
+test_injection_points_create(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ char *mode = text_to_cstring(PG_GETARG_TEXT_PP(1));
+ InjectionPointCallback callback;
+
+ if (strcmp(mode, "error") == 0)
+ callback = test_injection_error;
+ else if (strcmp(mode, "notice") == 0)
+ callback = test_injection_notice;
+ else
+ elog(ERROR, "incorrect mode \"%s\" for injection point creation", mode);
+
+ InjectionPointCreate(name, callback);
+
+ PG_RETURN_VOID();
+}
+
+/*
+ * SQL function for triggering an injection point.
+ */
+PG_FUNCTION_INFO_V1(test_injection_points_run);
+Datum
+test_injection_points_run(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ INJECTION_POINT_RUN(name);
+
+ PG_RETURN_VOID();
+}
+
+/*
+ * SQL function for dropping an injection point.
+ */
+PG_FUNCTION_INFO_V1(test_injection_points_drop);
+Datum
+test_injection_points_drop(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ InjectionPointDrop(name);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/test/modules/test_injection_points/test_injection_points.control b/src/test/modules/test_injection_points/test_injection_points.control
new file mode 100644
index 00000000000..a13657cfc65
--- /dev/null
+++ b/src/test/modules/test_injection_points/test_injection_points.control
@@ -0,0 +1,4 @@
+comment = 'Test code for injection points'
+default_version = '1.0'
+module_pathname = '$libdir/test_injection_points'
+relocatable = true
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index e8dd262152e..de1b4a627d3 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3530,6 +3530,9 @@ extern void InjectionPointDrop(const char *name);
<option>--enable-injection-points</option> from
<command>configure</command> or <option>-Dinjection_points=true</option>
from <application>Meson</application>.
+ An example can be found in
+ <filename>src/test/modules/test_injection_points</filename> in the
+ PostgreSQL source tree.
</para>
</sect2>
--
2.42.0
[application/pgp-signature] signature.asc (833B, ../../[email protected]/4-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Adding facility for injection points (or probe points?) for more advanced tests
@ 2023-10-25 04:36 Amul Sul <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Amul Sul @ 2023-10-25 04:36 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]>
On Wed, Oct 25, 2023 at 9:43 AM Michael Paquier <[email protected]> wrote:
> Hi all,
>
> I don't remember how many times in the last few years when I've had to
> hack the backend to produce a test case that involves a weird race
> condition across multiple processes running in the backend, to be able
> to prove a point or just test a fix (one recent case: 2b8e5273e949).
> Usually, I come to hardcoding stuff for the following situations:
> - Trigger a PANIC, to force recovery.
> - A FATAL, to take down a session, or just an ERROR.
> - palloc() failure injection.
> - Sleep to slow down a code path.
> - Pause and release with condition variable.
+1 for the feature.
TWIMW, here[1] is an interesting talk from pgconf.in 2020 on the similar
topic.
1] https://pgconf.in/conferences/pgconfin2020/program/proposals/101
Regards,
Amul Sul
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Adding facility for injection points (or probe points?) for more advanced tests
@ 2023-10-25 04:57 Michael Paquier <[email protected]>
parent: Amul Sul <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2023-10-25 04:57 UTC (permalink / raw)
To: Amul Sul <[email protected]>; +Cc: Postgres hackers <[email protected]>
On Wed, Oct 25, 2023 at 10:06:17AM +0530, Amul Sul wrote:
> +1 for the feature.
>
> TWIMW, here[1] is an interesting talk from pgconf.in 2020 on the similar
> topic.
>
> 1] https://pgconf.in/conferences/pgconfin2020/program/proposals/101
Right, this uses a shared hash table. There is a patch from 2019 that
summarizes this presentation as well:
https://www.postgresql.org/message-id/CANXE4TdxdESX1jKw48xet-5GvBFVSq%3D4cgNeioTQff372KO45A%40mail.g...
A different idea is that this patch could leverage a bgworker instead
of having a footprint in the postmaster. FWIW, I think that my patch
is more flexible than the modes added by faultinjector.h (see 0001),
because the actions that can be taken should not be limited by the
core code: the point registered could just use what it wants as
callback, so an extension could register a custom thing as well.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-10-25 04:57 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 13:07 [PATCH 8/8] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]>
2023-10-25 04:13 Adding facility for injection points (or probe points?) for more advanced tests Michael Paquier <[email protected]>
2023-10-25 04:36 ` Re: Adding facility for injection points (or probe points?) for more advanced tests Amul Sul <[email protected]>
2023-10-25 04:57 ` Re: Adding facility for injection points (or probe points?) for more advanced tests Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox