Subject: injection_points: Switch wait/wakeup to use atomics rather than latches

From: Michael Paquier <[email-masked]>
Date: Thu, 28 May 2026 11:43:29 +0900

Hi all,
(Adding Andrey in CC, as I'm sure he is interested in that.)

While looking at the test proposed on the thread about the ProcKill(),
I have been reminded about the fact that relying on latches and a
condition variable for the wait and the wakeups has its limits:
https://www.postgresql.org/message-id/aheVjCHmcbXBtiy0%40paquier.xyz

In this case, we are trying to synchronize backends once they don't
have latch assigned anymore, which defeats the purpose of wait/wakeup
because the condition variable used in injection_points while waiting
expects a Latch to be set for the processes we are waiting on.

Folks have complained about this limitation a couple of times in the
past, and I never got around to do something about it.  While looking
at that I have finished with the patch attached, which was
surprisingly simpler than what I thought was needed.  This replaces
the condition variable with a set of atomic counters.  The counters
are incremented at wakeup, and the wait checks them on a periodic
basis.  The wait loop uses a delay that increases over time, maxed at
100ms so as we can get a good responsiveness on fast machines, without
burning CPU for nothing in tests that require more wait time due to a
tight loop with the counter checks.

One thing worth noticing is the CHECK_FOR_INTERRUPTS() in the wait
loop, which is something we need for the autovacuum test in test_misc
that requires some signaling and interrupt processing.

It may make sense to be conservative and limit ourselves to do this
change on HEAD, but I'd like to suggest a backpatch down to v17 so as
future tests that rely on a such change can be backpatched.  I would
need this change for the other test, still consistency in the facility
primes for me here.

Note: The CI seems happy with the patch.

Thoughts or comments?
--
Michael
From 038b0f55dfe80f168fdc1b01b8cdadbf38fedfa2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 28 May 2026 11:15:33 +0900
Subject: [PATCH] injection_points: Switch wait/wakeup to rely on atomics

This removes the dependency based on counters and environment variables,
replacing the waiting loop by a wait on an atomic counter, whose check
increases over time in an exponential manner (starts at 10us, up to
100ms).
---
 .../injection_points/injection_points.c       | 53 ++++++++++---------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index ba282e3dcabf..9b8e1aaad0b0 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -23,11 +23,11 @@
 #include "miscadmin.h"
 #include "nodes/pg_list.h"
 #include "nodes/value.h"
-#include "storage/condition_variable.h"
 #include "storage/dsm_registry.h"
 #include "storage/ipc.h"
 #include "storage/lwlock.h"
 #include "storage/shmem.h"
+#include "storage/spin.h"
 #include "utils/builtins.h"
 #include "utils/guc.h"
 #include "utils/injection_point.h"
@@ -59,13 +59,10 @@ typedef struct InjectionPointSharedState
 	slock_t		lock;
 
 	/* Counters advancing when injection_points_wakeup() is called */
-	uint32		wait_counts[INJ_MAX_WAIT];
+	pg_atomic_uint32 wait_counts[INJ_MAX_WAIT];
 
 	/* Names of injection points attached to wait counters */
 	char		name[INJ_MAX_WAIT][INJ_NAME_MAXLEN];
-
-	/* Condition variable used for waits and wakeups */
-	ConditionVariable wait_point;
 } InjectionPointSharedState;
 
 /* Pointer to shared-memory state. */
@@ -102,9 +99,9 @@ injection_point_init_state(void *ptr, void *arg)
 	InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
 
 	SpinLockInit(&state->lock);
-	memset(state->wait_counts, 0, sizeof(state->wait_counts));
 	memset(state->name, 0, sizeof(state->name));
-	ConditionVariableInit(&state->wait_point);
+	for (int i = 0; i < INJ_MAX_WAIT; i++)
+		pg_atomic_init_u32(&state->wait_counts[i], 0);
 }
 
 static void
@@ -222,7 +219,7 @@ injection_notice(const char *name, const void *private_data, void *arg)
 		elog(NOTICE, "notice triggered for injection point %s", name);
 }
 
-/* Wait on a condition variable, awaken by injection_points_wakeup() */
+/* Wait until injection_points_wakeup() is called */
 void
 injection_wait(const char *name, const void *private_data, void *arg)
 {
@@ -254,31 +251,37 @@ injection_wait(const char *name, const void *private_data, void *arg)
 		{
 			index = i;
 			strlcpy(inj_state->name[i], name, INJ_NAME_MAXLEN);
-			old_wait_counts = inj_state->wait_counts[i];
+			old_wait_counts = pg_atomic_read_u32(&inj_state->wait_counts[i]);
 			break;
 		}
 	}
 	SpinLockRelease(&inj_state->lock);
 
 	if (index < 0)
-		elog(ERROR, "could not find free slot for wait of injection point %s ",
+		elog(ERROR, "could not find free slot for wait of injection point %s",
 			 name);
 
-	/* And sleep.. */
-	ConditionVariablePrepareToSleep(&inj_state->wait_point);
-	for (;;)
+	/*
+	 * Wait until the counter is bumped by injection_points_wakeup().
+	 *
+	 * This loop starts with a short delay for responsiveness, enlarged to
+	 * ease the CPU workload in slower environments.
+	 */
+#define INJ_WAIT_INITIAL_US		10	/* 10us */
+#define INJ_WAIT_MAX_US			100000	/* 100ms */
+	pgstat_report_wait_start(injection_wait_event);
 	{
-		uint32		new_wait_counts;
+		int			delay_us = INJ_WAIT_INITIAL_US;
 
-		SpinLockAcquire(&inj_state->lock);
-		new_wait_counts = inj_state->wait_counts[index];
-		SpinLockRelease(&inj_state->lock);
-
-		if (old_wait_counts != new_wait_counts)
-			break;
-		ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+		while (pg_atomic_read_u32(&inj_state->wait_counts[index]) == old_wait_counts)
+		{
+			CHECK_FOR_INTERRUPTS();
+			pg_usleep(delay_us);
+			if (delay_us < INJ_WAIT_MAX_US)
+				delay_us *= 2;
+		}
 	}
-	ConditionVariableCancelSleep();
+	pgstat_report_wait_end();
 
 	/* Remove this injection point from the waiters. */
 	SpinLockAcquire(&inj_state->lock);
@@ -443,7 +446,7 @@ injection_points_wakeup(PG_FUNCTION_ARGS)
 	if (inj_state == NULL)
 		injection_init_shmem();
 
-	/* First bump the wait counter for the injection point to wake up */
+	/* Find the injection point then bump its wait counter */
 	SpinLockAcquire(&inj_state->lock);
 	for (int i = 0; i < INJ_MAX_WAIT; i++)
 	{
@@ -458,11 +461,9 @@ injection_points_wakeup(PG_FUNCTION_ARGS)
 		SpinLockRelease(&inj_state->lock);
 		elog(ERROR, "could not find injection point %s to wake up", name);
 	}
-	inj_state->wait_counts[index]++;
 	SpinLockRelease(&inj_state->lock);
 
-	/* And broadcast the change to the waiters */
-	ConditionVariableBroadcast(&inj_state->wait_point);
+	pg_atomic_fetch_add_u32(&inj_state->wait_counts[index], 1);
 	PG_RETURN_VOID();
 }
 
-- 
2.54.0

---

From: Robert Haas <[email-masked]>
Date: Thu, 28 May 2026 08:40:39 -0400

> https://www.postgresql.org/message-id/aheVjCHmcbXBtiy0%40paquier.xyz

After reading this email, the linked-to email, and the commit message
for the patch, I still don't have a clear understanding of what this
is intended to fix. It seems like it's going to make the
responsiveness worse. In general, we want to replace escalating wait
loops with things that wake up instantly at the right time, and this
is going in the opposite direction.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Michael Paquier <[email-masked]>
Date: Fri, 29 May 2026 08:19:42 +0900

> is going in the opposite direction.

This is an exchange between responsiveness of the system and
flexibility.  I have had two complaints in the past about the fact
that the waits and wakeups were not doable due to the fact that we
rely on condition variables and latches:
- Postmaster context (lack of dsm access as one).  Heikki has
mentioned that to me once as annoying when hacking on tests there at
protocol level, at least.
- Second case as shown on the previous thread, which was a tricky
scenario involving the termination of backends.

One limitation is also related to wait event visibility, which may not
be visible in pg_stat_activity.  We could simply add a LOG entry in
injection_wait() once the old count is read, and rely on a server log
lookup in the TAP tests where we cannot use pg_stat_activity.

Compared to redesigning all the facilities that injection_points
relies on, this patch was striking me as having a good balance in
terms of responsiveness (min 10us, max 100ms) vs portability.  The
minimum threshold does not really matter much in terms of runtime on
fast machines.

Does this explanation make sense?
--
Michael

---

From: Robert Haas <[email-masked]>
Date: Fri, 29 May 2026 08:48:58 -0400

> rely on condition variables and latches:

I'm still struggling to understand. Condition variables and latches
are both designed to allow for nice waits and wakeups.

---

From: Heikki Linnakangas <[email-masked]>
Date: Fri, 29 May 2026 16:31:39 +0300

> are both designed to allow for nice waits and wakeups.

They only work after you have a PGPROC slot. If you want to inject code 
to authentication, or into postmaster, you cannot use them.

- Heikki

---

From: Robert Haas <[email-masked]>
Date: Fri, 29 May 2026 12:00:46 -0400

> to authentication, or into postmaster, you cannot use them.

OK, got it now.

---

From: Michael Paquier <[email-masked]>
Date: Sat, 30 May 2026 13:13:47 +0900

>> to authentication, or into postmaster, you cannot use them.

It seems like Heikki's comment was better worded than mine.

Also mentioned upthread, but the lack of PGPROC also means a lack of
monitoring as wait events cannot be tracked.  Currently, we rely on
that in the TAP tests.  For cases where the procs are not available, I
don't have a better idea than generating a LOG entry after the wait
counts have been generated (with a PID) and couple that with a poll of
the server logs to let a script understand that a process is in
waiting mode.

As a whole, I don't think that we should try to be fancy with the
implementation, which is why I have used primitives that should work
in any context, and I'm not convinced that this is worth its own
facility if that just means more responsiveness (in most cases the
wait should not take more than a couple ms to notice a wakeup).  I'm
open to more fancy ideas, of course.
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Sat, 30 May 2026 13:05:25 +0500

> On 28 May 2026, at 07:43, Michael Paquier <[email-masked]> wrote:
> 
> Andrey in CC, as I'm sure he is interested in that.

Thanks! That's exactly what I need for my tests.

> On 29 May 2026, at 18:31, Heikki Linnakangas <[email-masked]> wrote:
> 
> They only work after you have a PGPROC slot. If you want to inject code to authentication, or into postmaster, you cannot use them.

I have another reason: postmaster death behavior. When we wait on
ConVar and postmaster is kill-9-ed, we release all LWLocks. Which causes
corruption [0], because checkpointer can flush something that's not in WAL.

So I'm trying to build corruption-seeking tests using tool that can induce corruption
in tests.

About the patch:

Can we move pg_atomic_fetch_add_u32() back under the lock?
We determine slot index under lock, then wakeup slot outside the lock.
In a correctly written test meaning this is not a problem.
However, technically, identity of a slot can change between releasing the lock
and incrementing wait_counts[index].

I'll do another pass tomorrow, maybe something else will catch my eye.

Best regards, Andrey Borodin.

[0] https://www.postgresql.org/message-id/B3C69B86-7F82-4111-B97F-0005497BB745%40yandex-team.ru

---

From: Andrey Borodin <[email-masked]>
Date: Mon, 1 Jun 2026 16:25:40 +0500

> On 30 May 2026, at 13:05, Andrey Borodin <[email-masked]> wrote:
> 

I've tried the patch on my old corruption experiments. And it works for me.

I had to switch killing to something like
foreach my $i (1 .. 100)
{
my @alive = grep { kill 0, $_ } @cluster_pids;
last unless @alive;
kill 'KILL', @alive;
usleep(100_000);
}

The shared memory segment must be released before we attempt recovery.
But that's exactly what I wanted anyway. Thank you!

---

From: Michael Paquier <[email-masked]>
Date: Tue, 2 Jun 2026 13:15:55 +0900

> But that's exactly what I wanted anyway. Thank you!

How do you guarantee that a wait position is reached in the case where
you don't have access to wait events to make sure that the wait point
is reached?
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Tue, 2 Jun 2026 10:13:01 +0500

> On 2 Jun 2026, at 09:15, Michael Paquier <[email-masked]> wrote:
> 

In a research test? sleep(1)

---

From: Michael Paquier <[email-masked]>
Date: Tue, 2 Jun 2026 14:27:32 +0900

> In a research test? sleep(1)

A hardcoded sleep can work on a fast machine but it makes the test
slow.  A sleep is not a reliable technique if running the tests on a
slow machine, as an expected wait point may not have been reached.  We
have both very slow and very fast animals in the buildfarm.

Rewording my question a bit: did you consider some options regarding
what an equivalent of a wait event lookup should look like when we
don't have a PGPROC?  My idea of printing a LOG and do a server log
lookup would work, just asking if others have better ideas than the
only one I got.
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Tue, 2 Jun 2026 23:46:52 +0500

> On 2 Jun 2026, at 10:27, Michael Paquier <[email-masked]> wrote:
> 

For tests without PGPROC we can mmap() inj_state to a fixed file in
PGDATA/injection_points.shm. TAP can poll name[] to detect that a wait
point was reached and bump wait_counts[] to wake.

---

From: Michael Paquier <[email-masked]>
Date: Wed, 3 Jun 2026 07:23:16 +0900

> point was reached and bump wait_counts[] to wake.

That's a direction.  Only mmap() would not be sufficient, as WIN32 has
its own non-POSIX idea on the matter with CreateFileMapping() &
friends.  I am wondering if we should think harder about an interface
that could make such things easier for extensions.  Or we could have a
portable layer added to injection_points, as well..
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Fri, 12 Jun 2026 12:02:50 +0500

Hi Michael,

> On 3 Jun 2026, at 03:23, Michael Paquier <[email-masked]> wrote:
>
> friends.

Right, mmap() alone is not enough.  I hacked up a prototype (on top of
your atomics commit) that maps the state portably on both sides: POSIX
mmap() in the backend and a file-backed CreateFileMapping() on WIN32,
and the same in a small standalone client.  Perl cannot mmap()
portably, so the mapping is done by a tiny C helper that TAP drives,
rather than from the test script itself.

> I am wondering if we should think harder about an interface that could
> make such things easier for extensions.  Or we could have a portable
> layer added to injection_points, as well..

While prototyping I hit a constraint that I think answers this.  To arm
a point without SQL it is not enough to reach this module's wait state:
you have to reach the registry of active points, i.e.
ActiveInjectionPoints in injection_point.c.  INJECTION_POINT() consults
that array, and the module only supplies the callback once the core has
found the name.  So the part that has to be reachable from outside lives
in the core, not in the module.

That splits the problem into two layers rather cleanly:

  - core: the active-points array is backed by a file
    (injection_points.shm in the data directory).  This is the generic
    piece - any extension could arm or inspect points out of band, and
    it is what makes attach-without-SQL possible.  The lock-free
    generation protocol that reads the array is unchanged, so external
    readers can rely on it too.

  - module: injection_points keeps its own file
    (injection_points_wait.shm) for the wait/wakeup coordination of the
    "wait" action.  That part is test-specific and stays out of the
    core.

A standalone client (injection_points_state) maps both files the way
the backend does and can attach/detach a point and detect+release a
waiter, with no backend connection and no PGPROC.  A TAP test runs the
whole flow without SQL for arming or waking: arm a wait point with the
client, trigger it from a background session, let the client poll the
mapped file until the point is reached (so no sleep, and it behaves the
same on slow and fast animals), disarm it, then wake.  SQL is used only
to trigger the point and to observe injection_points_list().

This is a rough prototype to make the discussion concrete, not a
proposal of the final shape.  To name few open points:

1. Where the portable mapping helper should live.
   1. Keep the create-or-attach-file helper (POSIX + WIN32) local to
      injection_point.c, as in the prototype.  Simplest, no new public
      surface.
   2. Factor a small portable "named file-backed shared region" that any
      extension could use - an "interface for extensions".  More
      general, but a larger commitment and more to review.
   I have a slight preference for 1 now, growing into 2 only once a
   second user actually appears.

2. One file or two.  The prototype keeps the core registry and the
   module's wait coordination in two files.  Folding a wait counter into
   the core InjectionPointEntry would make it a single file, but it
   pushes test-only wait semantics into the core struct, which I would
   rather avoid.  Slight preference for two files.

3. The client writes the registry locklessly (no InjectionPointLock), so
   it assumes nothing else attaches/detaches the same array
   concurrently.  That holds for arming points out of band around a
   controlled session, but it is not a general concurrent-safe writer.
   I think that is acceptable for tests, but I am not sure.

4. The path defaults to injection_points.shm under the data directory,
   with an env override (PG_INJECTION_POINTS_FILE) so a point could be
   armed before the data dir exists or in single player mode.  I have
   not added a test for that, so probably it does not work. Yet.

5. The external mirror reads and writes the pg_atomic_uint{32,64} fields
   as plain integers.  That only holds where those atomics are a bare
   value; on a platform without native 64-bit atomics pg_atomic_uint64
   falls back to a spinlock-protected struct with a different layout, so
   the byte mirror would not match (and the backend's width assertion
   would fail to build there in the first place).  Even where they are
   native, the 64-bit field forces 8-byte alignment that the mirror must
   replicate - I got bitten by exactly that on the 32-bit build, where
   the entries array otherwise starts 4 bytes too early.  So a robust
   portable layer, if we want one, is not as trivial as it first looked
   to me.

6. Whether the external interface should do more than "wait".  The
   file-backed registry already lets an outside process attach a point
   to any callback (error, notice, ...); the only thing we can observe
   back without SQL today is that a wait was reached.  A natural
   generalization is a per-point hit counter that any process bumps when
   it runs the point, readable from outside.  That would let a test
   assert "initdb really went through this path", or "the postmaster
   reached this point before it failed at startup" - cases where there
   is no backend to query.  I have not built it, and it leans the same
   way as 2. (it would put an observation counter into the core entry),
   but it seems like the obvious next use-case if we go this route.

WDYT?  I am not at all sure the file-backed registry is the right
long-term shape; if you prefer the elog-and-grep route I am happy to drop
this, it was mostly a way to see how invasive the alternative really is.

Thank you!

P.S.  Fun observation - it speeds up tests. I swapped the "point reached"
detection in test_misc/010_index_concurrently_upsert.pl (41 cases, each polling
pg_stat_activity every 100ms via a fresh psql) for a 10ms poll of the mapped
file through the client.  Median test wall time dropped from 2.2s to 1.6s
(~27%) on my MacBook Air M5.

---

From: Andrey Borodin <[email-masked]>
Date: Sun, 14 Jun 2026 14:23:54 +0500

> On 12 Jun 2026, at 12:02, Andrey Borodin <[email-masked]> wrote:
> 
> I hacked up a prototype (on top of
> your atomics commit) that maps the state portably on both sides

v2026-06-12 passed locally but tripped on the Windows CI (EXEC_BACKEND),
v2026-06-14 fixes that.

The mistake was handing ActiveInjectionPoints to children as the
postmaster's pointer through BackendParameters.  That only works for the
main shared memory segment, which is re-attached at a fixed address; the
file-backed array is mapped wherever each process lands, so the inherited
pointer was garbage and the first point hit in a child crashed.

v2026-06-14 drops that and lets each process map the file itself. Rather
than racing to map it at the right moment in child startup, the registry
is mapped lazily on first use: the first time a process checks a point it
attaches the file if it exists, and treats "no file" as "nothing armed".
So a point is reached even when it fires before the child has attached
shared memory - e.g. "backend-initialize", which 005_negotiate_encryption
exercises - and a point armed out of band, even before the server is up,
is not silently missed.  Forked children still just inherit the
postmaster's mapping.

Personally I do not like lazy initialization, it's race-prone. But it
guarantees the file is mapped before the first point is checked, wherever
that point sits.

The same revision also closes an unrelated init race: when two processes
create the backing file at once, the loser could map it before the winner
sized it (SIG-something on first touch). The attach path now waits for the
file to reach full size before mapping.

CI is green. Same two patches on top of your atomics commit. Let me know
if this prototype goes radically wrong way.

Thanks!

---

From: Michael Paquier <[email-masked]>
Date: Mon, 15 Jun 2026 09:14:31 +0900

> if this prototype goes radically wrong way.

You may find my reply surprising (or not), but using a client tool to
bypass the SQL protocol is super invasive in terms of the in-core
changes, and I'm -1 on that.

Test tooling should rely on simple facilities, and you are
re-implementing quite a few things here that come with a new class of
bugs and what look like design issues to me due to the invasiveness:
- Reimplementation of the registry protocol for file mapping.  That
may be useful for other things, and there may be use for a refactored
in-core API, but I don't see why we should use it here:
- Mirroring of internal structs for shmem manipulation.
- Core data updates with zero locking.

I am wondering if we are not overcomplicating things here..  How about
this idea instead of 0002 and 0003, for paths that cannot rely on some
SQL:
- Let's use a file-based markup, located in a injection_points/ in the
data folder.
- When attaching a wait point, write a marker, say wait_$POINT.
- On wakeup, write a wakeup_$POINT. 
- The wait routine does periodic checks of the wakeup_$POINT file,
using a stat().

It depends on how much we want to achieve, but forcing a postmaster to
wait at an early startup sequence would then be something like:
- Add the a wait markup.
- At startup, shared_preload_libraries scans the pg_injection_points/
repo, fills in its shmem state by calling InjectionPointAttach().
- postmaster hits the injection point, calls injection_wait()
- Test does what it wants while the postmaster waits, manipulates
states.
- Test script drops a wakeup file.
- Postmaster sees the file, continues its startup sequence.
This means one cannot set an injection point until
shared_preload_libraries is loaded, of course.  Cleanup logic feels
kind of nice: test cleans his stuff, or just remove
pg_injection_points/ at shutdown with an exit callback.  No platform
specific logic, only FS checks.

This means a bit higher latency, but it eliminates a full class of
bugs due to the fact that it is simpler.  This would need to live
alongside patch 0001 so as a _PG_init() can be loaded when the
injection_points lib is loaded; we're still going to need it to fill
the shmem info with the wait slot being occupied.
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Tue, 7 Jul 2026 14:51:04 +0500

Hi Michael,

You suggested driving the module's wait points through the filesystem
instead of mapping shared memory out of process.  Here is that, rebased
on your atomics commit.  It is a test-module-only change; the core
injection point registry is untouched.  This message and the patch is
AI-editorialized for readability.

Design
------

Control dirs and files live under pg_injection_points/ in the data
directory:

    pg_injection_points/<point>/       attaches <point> as a wait point
    pg_injection_points/<point>/<pid>  that backend is waiting at <point>

The module scans this directory once at postmaster startup and attaches
a wait point for each subdirectory, so a test can arm a point before the
server is up (shared_preload_libraries is required for that).  When a
process reaches the point it creates its <pid> file and waits, polling
with stat() until the file is removed, in addition to the existing
wakeup counter.  So an out-of-process test can:

  - arm a point with mkdir, before SQL is available;
  - see which backends are blocked by listing the directory;
  - wake one specific backend by unlinking its <pid> file.

No SQL connection, no platform-specific code, and no out-of-process
access to shared memory.

The scan is deliberately one-shot at startup.  If a consumer ever needs
to (re)load points from the filesystem at runtime, that can be added
later as a dedicated injection point action (say a "reload" type); I
think we grow the facility only as the tests actually require.

The test question
-----------------

But in this project we grow the facility only when a test actually needs
it, so the real question is which test justifies this one.

The first case that seems to call for it is the ProcKill lock-group /
procLatch recycle race [0].  On that thread you concluded that
a wait point inside ProcKill() cannot use the latch-based wait, because
the fix (84b9d6bceab6) now disowns the latch earlier, and that the test
had to lean on statement_timeout to keep the leader parked long enough
- which is an anti-pattern.  You (IDK, maybe someone else) suggested
switching the wait to a latch-free shmem flag on HEAD; that is your atomics
commit, and this patch builds on it.

What the filesystem adds on top is the observability that was still
awkward there.  Instead of statement_timeout and an unreliable
pg_stat_activity (or query_until banners a la 011_lock_stats.pl), the
controller just waits for pg_injection_points/<point>/<pid> to appear
and then wakes that exact PID.  It also sidesteps the
before_shmem_exit(injection_points_cleanup) hook that detaches a
self-attached point before ProcKill runs, since the point is armed by
the startup scan rather than by the victim.

So the question is whether to rewrite that reproducer on top of these
two patches.  This design goes far beyond the minimal
plan you sketched (latch-free wait, attach() with an optional PID, then
the test), and that you were not sure the ProcKill case alone justifies
the churn.  Before writing it I would confirm it really needs the
filesystem parts and is not better served by something simpler.

Alternatively, we can use new capability for some other tests.

WDYT?

Thank you!

[0] https://www.postgresql.org/message-id/[email-masked]

---

From: Michael Paquier <[email-masked]>
Date: Wed, 8 Jul 2026 15:29:44 +0900

> Alternatively, we can use new capability for some other tests.

The hard part is that the implementation choices are driven by the
needs, where we may want to be able to do the following things without
having to touch SQL:
- Register a wait() at very early stages.
- Know from a client perspective that a PID is waiting, as we may not
have access to pg_stat_activity.
- Trigger a wakeup.
Your patch is able to achieve all of that, but it may be better to
know better about more use cases folks have seen before taking any
hard decision.

Here, one good case that I could see in the tree is 007_pre_auth.pl,
that uses currently as a workaround a background connection to create
a wait point.  We could switch that to register a wait early, but the
impact is limited.

@Heikki, what kind of ideas did you have for some of your toy tests
recently, particularly with the protocol area?  The protocol tests now
in the tree use errors (backend-initialize, GSSAPI and SSL startup
points), not waits and/or wakeups.

Applied the patch that removes the condition variable dependency, btw.
that's one thing less to worry about.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 8 Jul 2026 10:17:39 +0300

> points), not waits and/or wakeups.

I've got nothing in mind right now, but I remember when we started to 
talk about this, I was working on something where I wanted to inject 
waits before authentication. I think it was related to dead-end 
backends, max_connections, reserved_connections and all that.

- Heikki

---

From: Andrey Borodin <[email-masked]>
Date: Thu, 9 Jul 2026 00:17:20 +0500

> On 8 Jul 2026, at 12:17, Heikki Linnakangas <[email-masked]> wrote:
> 
> dead-end backends, max_connections, reserved_connections and all that

Hi Heikki, Michael,

Thanks for pushing the atomics patch.  Following Heikki's pointer to
waits before authentication and the connection-limit area, here is where
I looked:

- 002_connection_limits.pl did not seem to need a blocking wait, closer to the
  WARNING/LOG point (as discussed on the postmaster-cleanup thread [0]).

- 007_pre_auth.pl already attaches init-pre-auth, but it exists to test
  pg_stat_activity itself, and its old race (matching state='starting'
  before the backend reached the point) is already fixed by querying
  wait_event.  So converting it buys little, as you noted.

- So PFA a new test: a connection counts against max_connections
  from InitProcessPhase2(), before authentication.  I added a TAP test
  that fills every slot with pre-auth backends and checks that one more
  connection is refused with "too many clients".  Such backends can't be
  identified over SQL and there is no free slot for a monitoring session,
  so the test finds them through the filesystem markers. If this test
  direction seems viable - I'll polish the test.

Is this enough to justify the filesystem layer, or would you rather see
more cases?  Heikki, if you have a specific dead-end-backend /
reserved-connection scenario in mind, I'm happy to aim the test there.

It would be slightly better to have an unfixed bug to close with such
test.  But I do not have a spare one, so of course we could introduce a
new bug. But that clearly deserves its own thread.

Thanks!

[0] https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw