On Wed, Jul 10, 2024 at 01:16:15PM +0900, Michael Paquier wrote: > You mean with something that does a injection_point_cache_get() > followed by a callback run if anything is found in the local cache? > Why not. Based on what you have posted at [1], it looks like this had > better check the contents of the cache's generation with what's in > shmem, as well as destroying InjectionPointCache if there is nothing > else, so there's a possible dependency here depending on how much > maintenance this should do with the cache to keep it consistent.
Now that 86db52a5062a is in the tree, this could be done with a shortcut in InjectionPointCacheRefresh(). What do you think about something like the attached, with your suggested naming? -- Michael
From e33ab4202cf5e6ee79eef1927978d45e1cfa6034 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 16 Jul 2024 13:07:58 +0900
Subject: [PATCH] Add INJECTION_POINT_CACHED()
This works in combination with INJECTION_POINT_LOAD(), ensuring that an
injection point can be run without touching the shared memory area
related to injection points.
---
src/include/utils/injection_point.h | 3 ++
src/backend/utils/misc/injection_point.c | 33 ++++++++++++++++---
.../expected/injection_points.out | 13 ++++++++
.../injection_points--1.0.sql | 10 ++++++
.../injection_points/injection_points.c | 14 ++++++++
.../injection_points/sql/injection_points.sql | 2 ++
doc/src/sgml/xfunc.sgml | 6 +++-
7 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index bd3a62425c..a385e3df64 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -17,9 +17,11 @@
#ifdef USE_INJECTION_POINTS
#define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
#define INJECTION_POINT(name) InjectionPointRun(name)
+#define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
#else
#define INJECTION_POINT_LOAD(name) ((void) name)
#define INJECTION_POINT(name) ((void) name)
+#define INJECTION_POINT_CACHED(name) ((void) name)
#endif
/*
@@ -38,6 +40,7 @@ extern void InjectionPointAttach(const char *name,
int private_data_size);
extern void InjectionPointLoad(const char *name);
extern void InjectionPointRun(const char *name);
+extern void InjectionPointCached(const char *name);
extern bool InjectionPointDetach(const char *name);
#endif /* INJECTION_POINT_H */
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 84ad5e470d..9dd6575a9a 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -415,10 +415,11 @@ InjectionPointDetach(const char *name)
* Common workhorse of InjectionPointRun() and InjectionPointLoad()
*
* Checks if an injection point exists in shared memory, and update
- * the local cache entry accordingly.
+ * the local cache entry accordingly. If "direct" is true, return the
+ * point after looking up for it only in the cache.
*/
static InjectionPointCacheEntry *
-InjectionPointCacheRefresh(const char *name)
+InjectionPointCacheRefresh(const char *name, bool direct)
{
uint32 max_inuse;
int namelen;
@@ -461,6 +462,13 @@ InjectionPointCacheRefresh(const char *name)
cached = NULL;
}
+ /*
+ * If using a direct lookup, forget about the shared memory array
+ * and return immediately with the data found in the cache.
+ */
+ if (direct)
+ return cached;
+
/*
* Search the shared memory array.
*
@@ -531,7 +539,7 @@ void
InjectionPointLoad(const char *name)
{
#ifdef USE_INJECTION_POINTS
- InjectionPointCacheRefresh(name);
+ InjectionPointCacheRefresh(name, false);
#else
elog(ERROR, "Injection points are not supported by this build");
#endif
@@ -546,7 +554,24 @@ InjectionPointRun(const char *name)
#ifdef USE_INJECTION_POINTS
InjectionPointCacheEntry *cache_entry;
- cache_entry = InjectionPointCacheRefresh(name);
+ cache_entry = InjectionPointCacheRefresh(name, false);
+ if (cache_entry)
+ cache_entry->callback(name, cache_entry->private_data);
+#else
+ elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
+/*
+ * Execute an injection point directly from the cache, if defined.
+ */
+void
+InjectionPointCached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+ InjectionPointCacheEntry *cache_entry;
+
+ cache_entry = InjectionPointCacheRefresh(name, true);
if (cache_entry)
cache_entry->callback(name, cache_entry->private_data);
#else
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 2f60da900b..f25bbe4966 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -129,6 +129,12 @@ SELECT injection_points_detach('TestInjectionLog2');
(1 row)
-- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
+ injection_points_cached
+-------------------------
+
+(1 row)
+
SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
injection_points_load
-----------------------
@@ -147,6 +153,13 @@ SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
(1 row)
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
+NOTICE: notice triggered for injection point TestInjectionLogLoad
+ injection_points_cached
+-------------------------
+
+(1 row)
+
SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
NOTICE: notice triggered for injection point TestInjectionLogLoad
injection_points_run
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 e275c2cf5b..0f280419a5 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -34,6 +34,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_cached()
+--
+-- Executes the action attached to the injection point, from local cache.
+--
+CREATE FUNCTION injection_points_cached(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_cached'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_wakeup()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index b6c8e89324..15f9d0233c 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -333,6 +333,20 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for triggering an injection point from cache.
+ */
+PG_FUNCTION_INFO_V1(injection_points_cached);
+Datum
+injection_points_cached(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ INJECTION_POINT_CACHED(name);
+
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for waking up an injection point waiting in injection_wait().
*/
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index fabf0a8823..e3a481d604 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -42,9 +42,11 @@ SELECT injection_points_run('TestInjectionLog2'); -- notice
SELECT injection_points_detach('TestInjectionLog2');
-- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
SELECT injection_points_detach('TestInjectionLogLoad');
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 756a9d07fb..f02a48ead4 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3629,7 +3629,11 @@ INJECTION_POINT_LOAD(name);
doing all memory allocations at this stage without running the callback.
This is useful when an injection point is attached in a critical section
where no memory can be allocated: load the injection point outside the
- critical section, then run it in the critical section.
+ critical section, then run it in the critical section directly from
+ the process cache:
+<programlisting>
+INJECTION_POINT_CACHED(name);
+</programlisting>
</para>
<para>
--
2.45.2
signature.asc
Description: PGP signature
