Subject: Re: Adding facility for injection points (or probe points?) for more advanced tests

From: Nazir Bilal Yavuz <[email-masked]>
Date: Mon, 6 Nov 2023 22:28:14 +0300

Hi,

On Wed, 25 Oct 2023 at 07:13, Michael Paquier <[email-masked]> 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.

I liked the idea; thanks for working on this!

What do you think about creating a function for updating the already
created injection point's callback or name (mostly callback)? For now,
you need to drop and recreate the injection point to change the
callback or the name.

Here is my code correctness review:

diff --git a/meson_options.txt b/meson_options.txt
+option('injection_points', type: 'boolean', value: true,
+  description: 'Enable injection points')
+

It is enabled by default while building with meson.

diff --git a/src/backend/utils/misc/injection_point.c
b/src/backend/utils/misc/injection_point.c
+    LWLockRelease(InjectionPointLock);
+
+    /* If not found, do nothing? */
+    if (!found)
+        return;

It would be good to log a warning message here.

I tried to compile that with -Dwerror=true -Dinjection_points=false
and got some errors (warnings):

injection_point.c: In function ‘InjectionPointShmemSize’:
injection_point.c:59:1: error: control reaches end of non-void
function [-Werror=return-type]

injection_point.c: At top level:
injection_point.c:32:14: error: ‘InjectionPointHashByName’ defined but
not used [-Werror=unused-variable]

test_injection_points.c: In function ‘test_injection_points_run’:
test_injection_points.c:69:21: error: unused variable ‘name’
[-Werror=unused-variable]

The test_injection_points test runs and passes although I set
-Dinjection_points=false. That could be misleading, IMO the test
should be skipped if Postgres is not compiled with the injection
points.

Regards,
Nazir Bilal Yavuz
Microsoft

---

From: Michael Paquier <[email-masked]>
Date: Tue, 7 Nov 2023 17:01:16 +0900

> I liked the idea; thanks for working on this!

Thanks for the review.

> you need to drop and recreate the injection point to change the

I am not sure if that's worth the addition.  TBH, all the code I've
seen that would benefit from these APIs just set up a cluster,
register a few injection points with a module, and then run a set of
tests.  They can also remove points.  So I'm just aiming for simplest
for the moment.
 
> It is enabled by default while building with meson.

Indeed, fixed.

> It would be good to log a warning message here.

I don't think that's a good idea.  If a code path defines a
INJECTION_POINT_RUN() we'd get spurious warnings except if a point is
always defined when the build switch is enabled.

> and got some errors (warnings):

Right, fixed these three.

> should be skipped if Postgres is not compiled with the injection

The test suite has been using an alternate output, but perhaps you are
right that this has little value without the switch enabled anyway.
I've made the processing optional when the option is not used for
meson and ./configure (requires a variable in Makefile.global.in in
the latter case), removing the alternate output.

Please find v2.
--
Michael
From 4eaa22dae8888a452e4985c7419df153ad0dfbce Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Tue, 7 Nov 2023 17:00:49 +0900
Subject: [PATCH v2] 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      | 162 ++++++++++++++++++
 src/backend/utils/misc/meson.build            |   1 +
 src/test/modules/Makefile                     |   7 +
 src/test/modules/meson.build                  |   1 +
 .../modules/test_injection_points/.gitignore  |   4 +
 .../modules/test_injection_points/Makefile    |  22 +++
 .../expected/test_injection_points.out        |  45 +++++
 .../modules/test_injection_points/meson.build |  37 ++++
 .../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/installation.sgml                |  30 ++++
 doc/src/sgml/xfunc.sgml                       |  56 ++++++
 configure                                     |  34 ++++
 configure.ac                                  |   7 +
 meson.build                                   |   1 +
 meson_options.txt                             |   3 +
 src/Makefile.global.in                        |   1 +
 src/tools/pgindent/typedefs.list              |   1 +
 26 files changed, 599 insertions(+)
 create mode 100644 src/include/utils/injection_point.h
 create mode 100644 src/backend/utils/misc/injection_point.c
 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/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/include/pg_config.h.in b/src/include/pg_config.h.in
index d8a2985567..7a976821e5 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 0000000000..a0cb30a111
--- /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) ((void) 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 a3d8eacb8d..12b8a42fd9 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 f72f2906ce..42a048746d 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 d7995931bd..5631d29138 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 c2971c7678..d9f59785b9 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 0000000000..dd1cb6329a
--- /dev/null
+++ b/src/backend/utils/misc/injection_point.c
@@ -0,0 +1,162 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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.
+ */
+#ifdef USE_INJECTION_POINTS
+static HTAB *InjectionPointHashByName;	/* find points from names */
+#endif
+
+/* 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;
+#else
+	return 0;
+#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 f719c97c05..1438859b69 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/src/test/modules/Makefile b/src/test/modules/Makefile
index e81873cb5a..3839acc699 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -34,6 +34,13 @@ SUBDIRS = \
 		  unsafe_tests \
 		  worker_spi
 
+
+ifeq ($(enable_injection_points),yes)
+SUBDIRS += test_injection_points
+else
+ALWAYS_SUBDIRS += test_injection_points
+endif
+
 ifeq ($(with_ssl),openssl)
 SUBDIRS += ssl_passphrase_callback
 else
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index fcd643f6f1..67e9eff878 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 0000000000..5dcb3ff972
--- /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 0000000000..65bcdde782
--- /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 0000000000..c85639146c
--- /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/meson.build b/src/test/modules/test_injection_points/meson.build
new file mode 100644
index 0000000000..7509a102ef
--- /dev/null
+++ b/src/test/modules/test_injection_points/meson.build
@@ -0,0 +1,37 @@
+# Copyright (c) 2022-2023, PostgreSQL Global Development Group
+
+if not get_option('injection_points')
+  subdir_done()
+endif
+
+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 0000000000..44135c7c37
--- /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 0000000000..c4c42b5b69
--- /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 0000000000..a842d837e8
--- /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 0000000000..a13657cfc6
--- /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/installation.sgml b/doc/src/sgml/installation.sgml
index a3dc6eb855..d8840bce1f 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1666,6 +1666,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>
@@ -3163,6 +3178,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 89116ae74c..ded7b6c06b 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3510,6 +3510,62 @@ 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>.
+     An example can be found in
+     <filename>src/test/modules/test_injection_points</filename> in the
+     PostgreSQL source tree.
+    </para>
+   </sect2>
+
    <sect2 id="extend-cpp">
     <title>Using C++ for Extensibility</title>
 
diff --git a/configure b/configure
index c064115038..ce0ef0133d 100755
--- a/configure
+++ b/configure
@@ -759,6 +759,7 @@ CPPFLAGS
 LDFLAGS
 CFLAGS
 CC
+enable_injection_points
 enable_tap_tests
 enable_dtrace
 DTRACEFLAGS
@@ -839,6 +840,7 @@ enable_profiling
 enable_coverage
 enable_dtrace
 enable_tap_tests
+enable_injection_points
 with_blocksize
 with_segsize
 with_segsize_blocks
@@ -1532,6 +1534,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 +3686,36 @@ 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 f220b379b3..af13b6da69 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,13 @@ 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)])])
+AC_SUBST(enable_injection_points)
+
 #
 # Block size
 #
diff --git a/meson.build b/meson.build
index 88a9d9051f..fc26a02bb1 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 d2f95cfec3..8d0d35636d 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: false,
+  description: 'Enable injection points')
+
 option('PG_TEST_EXTRA', type: 'string', value: '',
   description: 'Enable selected extra tests')
 
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index b3ca6392a6..ceb5895d1b 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -203,6 +203,7 @@ enable_nls	= @enable_nls@
 enable_debug	= @enable_debug@
 enable_dtrace	= @enable_dtrace@
 enable_coverage	= @enable_coverage@
+enable_injection_points = @enable_injection_points@
 enable_tap_tests	= @enable_tap_tests@
 
 python_includespec	= @python_includespec@
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 87c1aee379..b069a84c61 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1137,6 +1137,7 @@ IdentLine
 IdentifierLookup
 IdentifySystemCmd
 IfStackElem
+InjectionPointEntryByName
 ImportForeignSchemaStmt
 ImportForeignSchemaType
 ImportForeignSchema_function
-- 
2.42.0

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 10 Nov 2023 14:44:25 -0600

>> I liked the idea; thanks for working on this!

+1, this seems very useful.

> +#define INJECTION_POINT_RUN(name) ((void) name)

nitpick: Why is the non-injection-point version "(void) name"?  I see
"(void) true" used elsewhere for this purpose.

> +    elog(NOTICE, "%s: executed custom callback", name);

Why do we provide the name to the callback functions?

Overall, the design looks pretty good to me.  I think it's a good idea to
keep it simple to start with.  Since this is really only intended for
special tests that run in special builds, it seems like we ought to be able
to change it easily in the future as needed.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Michael Paquier <[email-masked]>
Date: Mon, 13 Nov 2023 14:48:13 +0900

> "(void) true" used elsewhere for this purpose.

Or (void) 0.

> Why do we provide the name to the callback functions?

This is for the use of the same callback across multiple points, and
tracking the name of the event happening was making sense to me to
know which code path is being taken when a callback is called.  One
thing that I got in mind as well here is to be able to register custom
wait events based on the name of the callback taken, for example on a 
condition variable, a latch or a named LWLock.

> to change it easily in the future as needed.

Yes, my first idea is to keep the initial design minimal and take the
temperature.  As far as I can see, there seem to not be any strong
objection with this basic design, still I agree that I need to show a
bit more code about its usability.  I have some SQL and recovery cases
where this is handy and these have piled over time, including at least
two/three of them with more basic APIs in the test module may make
sense in the initial batch of what I am proposing here.
--
Michael