diff options
author | Amit Langote <amitlan@postgresql.org> | 2025-02-22 15:19:23 +0900 |
---|---|---|
committer | Amit Langote <amitlan@postgresql.org> | 2025-02-22 15:19:23 +0900 |
commit | 4f1b6e5bb4fe9bc74395d30d689b28e9cda654a5 (patch) | |
tree | 948fdd433e962517815fdaa77140343ea999efac /src/test/modules/delay_execution/delay_execution.c | |
parent | f8d7f29b3e81db59b95e4b5baaa6943178c89fd8 (diff) | |
download | postgresql-4f1b6e5bb4fe9bc74395d30d689b28e9cda654a5.tar.gz postgresql-4f1b6e5bb4fe9bc74395d30d689b28e9cda654a5.zip |
Remove unstable test suite added by 525392d57
The 'cached-plan-inval' test suite, introduced in 525392d57 under
src/test/modules/delay_execution, aimed to verify that cached plan
invalidation triggers replanning after deferred locks are taken.
However, its ExecutorStart_hook-based approach relies on lock timing
assumptions that, in retrospect, are fragile. This instability was
exposed by failures on BF animal trilobite, which builds with
CLOBBER_CACHE_ALWAYS.
One option was to dynamically disable the cache behavior that causes
the test suite to fail by setting "debug_discard_caches = 0", but it
seems better to remove the suite. The risk of future failures due to
other cache flush hazards outweighs the benefit of catching real
breakage in the backend behavior it tests.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2990641.1740117879@sss.pgh.pa.us
Diffstat (limited to 'src/test/modules/delay_execution/delay_execution.c')
-rw-r--r-- | src/test/modules/delay_execution/delay_execution.c | 66 |
1 files changed, 6 insertions, 60 deletions
diff --git a/src/test/modules/delay_execution/delay_execution.c b/src/test/modules/delay_execution/delay_execution.c index ad22bc9f2a8..7bc97f84a1c 100644 --- a/src/test/modules/delay_execution/delay_execution.c +++ b/src/test/modules/delay_execution/delay_execution.c @@ -1,18 +1,14 @@ /*------------------------------------------------------------------------- * * delay_execution.c - * Test module to introduce delay at various points during execution of a - * query to test that execution proceeds safely in light of concurrent - * changes. + * Test module to allow delay between parsing and execution of a query. * * The delay is implemented by taking and immediately releasing a specified * advisory lock. If another process has previously taken that lock, the * current process will be blocked until the lock is released; otherwise, * there's no effect. This allows an isolationtester script to reliably - * test behaviors where some specified action happens in another backend in - * a couple of cases: 1) between parsing and execution of any desired query - * when using the planner_hook, 2) between RevalidateCachedQuery() and - * ExecutorStart() when using the ExecutorStart_hook. + * test behaviors where some specified action happens in another backend + * between parsing and execution of any desired query. * * Copyright (c) 2020-2025, PostgreSQL Global Development Group * @@ -26,7 +22,6 @@ #include <limits.h> -#include "executor/executor.h" #include "optimizer/planner.h" #include "utils/fmgrprotos.h" #include "utils/guc.h" @@ -37,11 +32,9 @@ PG_MODULE_MAGIC; /* GUC: advisory lock ID to use. Zero disables the feature. */ static int post_planning_lock_id = 0; -static int executor_start_lock_id = 0; -/* Save previous hook users to be a good citizen */ +/* Save previous planner hook user to be a good citizen */ static planner_hook_type prev_planner_hook = NULL; -static ExecutorStart_hook_type prev_ExecutorStart_hook = NULL; /* planner_hook function to provide the desired delay */ @@ -77,45 +70,11 @@ delay_execution_planner(Query *parse, const char *query_string, return result; } -/* ExecutorStart_hook function to provide the desired delay */ -static bool -delay_execution_ExecutorStart(QueryDesc *queryDesc, int eflags) -{ - bool plan_valid; - - /* If enabled, delay by taking and releasing the specified lock */ - if (executor_start_lock_id != 0) - { - DirectFunctionCall1(pg_advisory_lock_int8, - Int64GetDatum((int64) executor_start_lock_id)); - DirectFunctionCall1(pg_advisory_unlock_int8, - Int64GetDatum((int64) executor_start_lock_id)); - - /* - * Ensure that we notice any pending invalidations, since the advisory - * lock functions don't do this. - */ - AcceptInvalidationMessages(); - } - - /* Now start the executor, possibly via a previous hook user */ - if (prev_ExecutorStart_hook) - plan_valid = prev_ExecutorStart_hook(queryDesc, eflags); - else - plan_valid = standard_ExecutorStart(queryDesc, eflags); - - if (executor_start_lock_id != 0) - elog(NOTICE, "Finished ExecutorStart(): CachedPlan is %s", - plan_valid ? "valid" : "not valid"); - - return plan_valid; -} - /* Module load function */ void _PG_init(void) { - /* Set up GUCs to control which lock is used */ + /* Set up the GUC to control which lock is used */ DefineCustomIntVariable("delay_execution.post_planning_lock_id", "Sets the advisory lock ID to be locked/unlocked after planning.", "Zero disables the delay.", @@ -128,22 +87,9 @@ _PG_init(void) NULL, NULL); - DefineCustomIntVariable("delay_execution.executor_start_lock_id", - "Sets the advisory lock ID to be locked/unlocked before starting execution.", - "Zero disables the delay.", - &executor_start_lock_id, - 0, - 0, INT_MAX, - PGC_USERSET, - 0, - NULL, - NULL, - NULL); MarkGUCPrefixReserved("delay_execution"); - /* Install our hooks. */ + /* Install our hook */ prev_planner_hook = planner_hook; planner_hook = delay_execution_planner; - prev_ExecutorStart_hook = ExecutorStart_hook; - ExecutorStart_hook = delay_execution_ExecutorStart; } |