From 5008207f28c68360ec3d466852697797994bb330 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Mon, 10 Feb 2025 13:19:54 +0000 Subject: [PATCH v1 1/2] Add injection_points_wakeup_detach() and modify injection_wait() This commit adds: - injection_points_wakeup_detach() to be able to wakeup() and detach() while ensuring that no process can wait in between (holding the spinlock during the whole duration). - A check in injection_wait() to search if an existing injection slot with the same name already exists (If so, reuse it). --- .../injection_points--1.0.sql | 10 +++ .../injection_points/injection_points.c | 65 ++++++++++++++++--- 2 files changed, 65 insertions(+), 10 deletions(-) 100.0% src/test/modules/injection_points/ 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 5d83f08811b..b4ae67fd97b 100644 --- a/src/test/modules/injection_points/injection_points--1.0.sql +++ b/src/test/modules/injection_points/injection_points--1.0.sql @@ -75,6 +75,16 @@ RETURNS void AS 'MODULE_PATHNAME', 'injection_points_detach' LANGUAGE C STRICT PARALLEL UNSAFE; +-- +-- injection_points_wakeup_detach() +-- +-- Wakes up and detaches the current action, if any, from the given injection point. +-- +CREATE FUNCTION injection_points_wakeup_detach(IN point_name TEXT) +RETURNS void +AS 'MODULE_PATHNAME', 'injection_points_wakeup_detach' +LANGUAGE C STRICT PARALLEL UNSAFE; + -- -- injection_points_stats_numcalls() -- diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index ad528d77752..97397449dfb 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -93,6 +93,9 @@ typedef struct InjectionPointSharedState /* Pointer to shared-memory state. */ static InjectionPointSharedState *inj_state = NULL; +static Datum injection_points_wakeup_internal(FunctionCallInfo fcinfo, bool lock, + bool have_to_wait); + extern PGDLLEXPORT void injection_error(const char *name, const void *private_data); extern PGDLLEXPORT void injection_notice(const char *name, @@ -294,19 +297,30 @@ injection_wait(const char *name, const void *private_data) SpinLockAcquire(&inj_state->lock); for (int i = 0; i < INJ_MAX_WAIT; i++) { - if (inj_state->name[i][0] == '\0') + /* + * It might be that a waiting process has been killed before being + * able to reset inj_state->name[i][0] to '\0', so checking if there + * is a slot with the same name. + */ + if (strcmp(name, inj_state->name[i]) == 0) { index = i; - strlcpy(inj_state->name[i], name, INJ_NAME_MAXLEN); - old_wait_counts = inj_state->wait_counts[i]; break; } + else if (inj_state->name[i][0] == '\0' && index < 0) + index = i; } + SpinLockRelease(&inj_state->lock); if (index < 0) elog(ERROR, "could not find free slot for wait of injection point %s ", name); + else + { + strlcpy(inj_state->name[index], name, INJ_NAME_MAXLEN); + old_wait_counts = inj_state->wait_counts[index]; + } /* And sleep.. */ ConditionVariablePrepareToSleep(&inj_state->wait_point); @@ -427,10 +441,12 @@ injection_points_cached(PG_FUNCTION_ARGS) /* * SQL function for waking up an injection point waiting in injection_wait(). + * If "lock" is true then the function handles the locking. + * If "have_to_wait" is true then the function returns an error if no process + * is waiting. */ -PG_FUNCTION_INFO_V1(injection_points_wakeup); -Datum -injection_points_wakeup(PG_FUNCTION_ARGS) +static Datum +injection_points_wakeup_internal(FunctionCallInfo fcinfo, bool lock, bool have_to_wait) { char *name = text_to_cstring(PG_GETARG_TEXT_PP(0)); int index = -1; @@ -439,7 +455,8 @@ injection_points_wakeup(PG_FUNCTION_ARGS) injection_init_shmem(); /* First bump the wait counter for the injection point to wake up */ - SpinLockAcquire(&inj_state->lock); + if (lock) + SpinLockAcquire(&inj_state->lock); for (int i = 0; i < INJ_MAX_WAIT; i++) { if (strcmp(name, inj_state->name[i]) == 0) @@ -450,17 +467,29 @@ injection_points_wakeup(PG_FUNCTION_ARGS) } if (index < 0) { - SpinLockRelease(&inj_state->lock); - elog(ERROR, "could not find injection point %s to wake up", name); + if (lock) + SpinLockRelease(&inj_state->lock); + if (have_to_wait) + elog(ERROR, "could not find injection point %s to wake up", name); + else + PG_RETURN_VOID(); } inj_state->wait_counts[index]++; - SpinLockRelease(&inj_state->lock); + if (lock) + SpinLockRelease(&inj_state->lock); /* And broadcast the change to the waiters */ ConditionVariableBroadcast(&inj_state->wait_point); PG_RETURN_VOID(); } +PG_FUNCTION_INFO_V1(injection_points_wakeup); +Datum +injection_points_wakeup(PG_FUNCTION_ARGS) +{ + return injection_points_wakeup_internal(fcinfo, true, true); +} + /* * injection_points_set_local * @@ -516,6 +545,22 @@ injection_points_detach(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * SQL function for waking up and dropping an injection point. + */ +PG_FUNCTION_INFO_V1(injection_points_wakeup_detach); +Datum +injection_points_wakeup_detach(PG_FUNCTION_ARGS) +{ + if (inj_state == NULL) + injection_init_shmem(); + + SpinLockAcquire(&inj_state->lock); + injection_points_wakeup_internal(fcinfo, false, false); + injection_points_detach(fcinfo); + SpinLockRelease(&inj_state->lock); + PG_RETURN_VOID(); +} void _PG_init(void) -- 2.34.1