Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4f1b6e5

Browse files
committed
Remove unstable test suite added by 525392d
The 'cached-plan-inval' test suite, introduced in 525392d 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
1 parent f8d7f29 commit 4f1b6e5

File tree

5 files changed

+7
-399
lines changed

5 files changed

+7
-399
lines changed

src/test/modules/delay_execution/Makefile

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ OBJS = \
88
delay_execution.o
99

1010
ISOLATION = partition-addition \
11-
partition-removal-1 \
12-
cached-plan-inval
11+
partition-removal-1
1312

1413
ifdef USE_PGXS
1514
PG_CONFIG = pg_config
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
/*-------------------------------------------------------------------------
22
*
33
* delay_execution.c
4-
* Test module to introduce delay at various points during execution of a
5-
* query to test that execution proceeds safely in light of concurrent
6-
* changes.
4+
* Test module to allow delay between parsing and execution of a query.
75
*
86
* The delay is implemented by taking and immediately releasing a specified
97
* advisory lock. If another process has previously taken that lock, the
108
* current process will be blocked until the lock is released; otherwise,
119
* there's no effect. This allows an isolationtester script to reliably
12-
* test behaviors where some specified action happens in another backend in
13-
* a couple of cases: 1) between parsing and execution of any desired query
14-
* when using the planner_hook, 2) between RevalidateCachedQuery() and
15-
* ExecutorStart() when using the ExecutorStart_hook.
10+
* test behaviors where some specified action happens in another backend
11+
* between parsing and execution of any desired query.
1612
*
1713
* Copyright (c) 2020-2025, PostgreSQL Global Development Group
1814
*
@@ -26,7 +22,6 @@
2622

2723
#include <limits.h>
2824

29-
#include "executor/executor.h"
3025
#include "optimizer/planner.h"
3126
#include "utils/fmgrprotos.h"
3227
#include "utils/guc.h"
@@ -37,11 +32,9 @@ PG_MODULE_MAGIC;
3732

3833
/* GUC: advisory lock ID to use. Zero disables the feature. */
3934
static int post_planning_lock_id = 0;
40-
static int executor_start_lock_id = 0;
4135

42-
/* Save previous hook users to be a good citizen */
36+
/* Save previous planner hook user to be a good citizen */
4337
static planner_hook_type prev_planner_hook = NULL;
44-
static ExecutorStart_hook_type prev_ExecutorStart_hook = NULL;
4538

4639

4740
/* planner_hook function to provide the desired delay */
@@ -77,45 +70,11 @@ delay_execution_planner(Query *parse, const char *query_string,
7770
return result;
7871
}
7972

80-
/* ExecutorStart_hook function to provide the desired delay */
81-
static bool
82-
delay_execution_ExecutorStart(QueryDesc *queryDesc, int eflags)
83-
{
84-
bool plan_valid;
85-
86-
/* If enabled, delay by taking and releasing the specified lock */
87-
if (executor_start_lock_id != 0)
88-
{
89-
DirectFunctionCall1(pg_advisory_lock_int8,
90-
Int64GetDatum((int64) executor_start_lock_id));
91-
DirectFunctionCall1(pg_advisory_unlock_int8,
92-
Int64GetDatum((int64) executor_start_lock_id));
93-
94-
/*
95-
* Ensure that we notice any pending invalidations, since the advisory
96-
* lock functions don't do this.
97-
*/
98-
AcceptInvalidationMessages();
99-
}
100-
101-
/* Now start the executor, possibly via a previous hook user */
102-
if (prev_ExecutorStart_hook)
103-
plan_valid = prev_ExecutorStart_hook(queryDesc, eflags);
104-
else
105-
plan_valid = standard_ExecutorStart(queryDesc, eflags);
106-
107-
if (executor_start_lock_id != 0)
108-
elog(NOTICE, "Finished ExecutorStart(): CachedPlan is %s",
109-
plan_valid ? "valid" : "not valid");
110-
111-
return plan_valid;
112-
}
113-
11473
/* Module load function */
11574
void
11675
_PG_init(void)
11776
{
118-
/* Set up GUCs to control which lock is used */
77+
/* Set up the GUC to control which lock is used */
11978
DefineCustomIntVariable("delay_execution.post_planning_lock_id",
12079
"Sets the advisory lock ID to be locked/unlocked after planning.",
12180
"Zero disables the delay.",
@@ -128,22 +87,9 @@ _PG_init(void)
12887
NULL,
12988
NULL);
13089

131-
DefineCustomIntVariable("delay_execution.executor_start_lock_id",
132-
"Sets the advisory lock ID to be locked/unlocked before starting execution.",
133-
"Zero disables the delay.",
134-
&executor_start_lock_id,
135-
0,
136-
0, INT_MAX,
137-
PGC_USERSET,
138-
0,
139-
NULL,
140-
NULL,
141-
NULL);
14290
MarkGUCPrefixReserved("delay_execution");
14391

144-
/* Install our hooks. */
92+
/* Install our hook */
14593
prev_planner_hook = planner_hook;
14694
planner_hook = delay_execution_planner;
147-
prev_ExecutorStart_hook = ExecutorStart_hook;
148-
ExecutorStart_hook = delay_execution_ExecutorStart;
14995
}

src/test/modules/delay_execution/expected/cached-plan-inval.out

-250
This file was deleted.

src/test/modules/delay_execution/meson.build

-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ tests += {
2424
'specs': [
2525
'partition-addition',
2626
'partition-removal-1',
27-
'cached-plan-inval',
2827
],
2928
},
3029
}

0 commit comments

Comments
 (0)