On Mon, Apr 08, 2024 at 10:22:40AM +0900, Michael Paquier wrote: > For now I have applied 997db123c054 to make the GIN tests with > injection points repeatable as it was an independent issue, and > f587338dec87 to add the local function pieces.
Bharath has reported me offlist that one of the new tests has a race
condition when doing the reconnection. When the backend creating the
local points is very slow to exit, the backend created after the
reconnection may detect that a local point previously created still
exists, causing a failure. The failure can be reproduced with a sleep
in the shmem exit callback, like:
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -163,6 +163,8 @@ injection_points_cleanup(int code, Datum arg)
if (!injection_point_local)
return;
+ pg_usleep(1000000 * 1L);
+
SpinLockAcquire(&inj_state->lock);
for (int i = 0; i < INJ_MAX_CONDITION; i++)
{
At first I was looking at a loop with a scan of pg_stat_activity, but
I've noticed that regress.so includes a wait_pid() that we can use to
make sure that a given process exits before moving on to the next
parts of a test, so I propose to just reuse that here. This requires
tweaks with --dlpath for meson and ./configure, nothing new. The CI
is clean. Patch attached.
Thoughts?
--
Michael
From 8f43807000c1f33a0238d7bbcc148a196e4134e4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Apr 2024 16:28:17 +0900
Subject: [PATCH] Stabilize injection point test
---
src/test/modules/injection_points/Makefile | 1 +
.../expected/injection_points.out | 16 ++++++++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../injection_points/sql/injection_points.sql | 15 +++++++++++++++
4 files changed, 33 insertions(+)
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 1cb395c37a..31bd787994 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -7,6 +7,7 @@ DATA = injection_points--1.0.sql
PGFILEDESC = "injection_points - facility for injection points"
REGRESS = injection_points
+REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
# The injection points are cluster-wide, so disable installcheck
NO_INSTALLCHECK = 1
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 3d94016ac9..d2a69b54e8 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -1,4 +1,11 @@
CREATE EXTENSION injection_points;
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
+CREATE FUNCTION wait_pid(int)
+ RETURNS void
+ AS :'regresslib'
+ LANGUAGE C STRICT;
SELECT injection_points_attach('TestInjectionBooh', 'booh');
ERROR: incorrect action "booh" for injection point creation
SELECT injection_points_attach('TestInjectionError', 'error');
@@ -156,8 +163,17 @@ NOTICE: notice triggered for injection point TestConditionLocal2
(1 row)
+select pg_backend_pid() as oldpid \gset
-- reload, local injection points should be gone.
\c
+-- Wait for the previous backend process to exit, ensuring that its local
+-- injection points are cleaned up.
+SELECT wait_pid(:'oldpid');
+ wait_pid
+----------
+
+(1 row)
+
SELECT injection_points_run('TestConditionLocal1'); -- nothing
injection_points_run
----------------------
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index a29217f75f..8e1b5b4539 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -33,6 +33,7 @@ tests += {
'sql': [
'injection_points',
],
+ 'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
# The injection points are cluster-wide, so disable installcheck
'runningcheck': false,
},
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index 2aa76a542b..e18146eb8c 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -1,5 +1,14 @@
CREATE EXTENSION injection_points;
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
+
+CREATE FUNCTION wait_pid(int)
+ RETURNS void
+ AS :'regresslib'
+ LANGUAGE C STRICT;
+
SELECT injection_points_attach('TestInjectionBooh', 'booh');
SELECT injection_points_attach('TestInjectionError', 'error');
SELECT injection_points_attach('TestInjectionLog', 'notice');
@@ -40,8 +49,14 @@ SELECT injection_points_attach('TestConditionLocal1', 'error');
SELECT injection_points_attach('TestConditionLocal2', 'notice');
SELECT injection_points_run('TestConditionLocal1'); -- error
SELECT injection_points_run('TestConditionLocal2'); -- notice
+
+select pg_backend_pid() as oldpid \gset
+
-- reload, local injection points should be gone.
\c
+-- Wait for the previous backend process to exit, ensuring that its local
+-- injection points are cleaned up.
+SELECT wait_pid(:'oldpid');
SELECT injection_points_run('TestConditionLocal1'); -- nothing
SELECT injection_points_run('TestConditionLocal2'); -- nothing
SELECT injection_points_run('TestConditionError'); -- error
--
2.43.0
signature.asc
Description: PGP signature
