Re: Allow a condition string in an injection point

On Mon, Aug 10, 2026 at 05:31:00PM -0500, Sami Imseih wrote:
> The facilities that this will be useful for are those in which the
> injection_point_set_local()
> cannot be used and thus we cannot clearly determine if the injection
> point is reached
> at the correct condition. [0] was one example, which led to this
> thread, but we dealt
> with this by using wait_for_log to ensure that we reached the correct code path.

0001 feels overall OK. I just have a few comments.

+-- injection point condition string matching.
+SELECT injection_points_attach('TestConditionString', 'notice', 'MyString');
+SELECT injection_points_run('TestConditionString', 'MyString'); -- notice
+SELECT injection_points_run('TestConditionString', 'WrongString'); -- nothing
+SELECT injection_points_run('TestConditionString', NULL); -- nothing
+SELECT injection_points_detach('TestConditionString');

This is a duplicate set of tests with the local case. Let's trim the
cases a bit a drop this part.

SELECT injection_points_run('TestConditionLocal2'); -- nothing
+SELECT injection_points_run('TestConditionLocalString', 'LocalData'); -- nothing

This one also to check that the local point has been dropped is not
really required to me: we already make sure that a bunch of the other
points have been detached.

CREATE FUNCTION injection_points_attach(IN point_name TEXT,
[ ... ]
-LANGUAGE C STRICT PARALLEL UNSAFE;
+LANGUAGE C PARALLEL UNSAFE;

Okay with the strictness here.

INJ_CONDITION_ALWAYS = 0, /* always run */
- INJ_CONDITION_PID, /* PID restriction */
+ INJ_CONDITION_PID = 1 << 0, /* PID restriction */
+ INJ_CONDITION_STRING = 1 << 1, /* generic string match against arg */

Hmm. Could it be better to rename "ALWAYS" to "NONE" then? That
would feel less confusing to me after switching to a bitmask as we may
want to apply multiple conditions.

> Looking at [1], if we want to test toast reloptions for autovacuum, we
> will need some type
> of way to ensure that we are inspecting the correct relations for the
> test. Since
> autovacuum cannot rely on injection_point_set_local because workers will not
> set the local injection point, we can use string conditions to ensure
> we reached the
> correct table. the nocfbot test attached shows how we can build such tests for
> autovacuum. The test fails without the latest patchset in [1]. CCing Nathan as
> his patch is being discussed.

+# A TOAST relation inherits vacuum reloptions from its main table when it has
+# no toast.* value of its own. Condition strings key each wait to the TOAST
+# relation, so its resolution is observed on its own.

Better perhaps to wait for Nathan's input on how this would help.
That depends on the resulting commit merged into the tree. But I
don't think that we should, see below for extra ideas.

> Without this condition string, to implement such a test, we will need
> a NOTICE to log
> and we will need to use regexp to examine the log to make sure our
> toast table fired
> at the correct injection point. With the condition string, we can
> simply just use
> wait_for_event since we know which table the injection point will fire on.

So your argument is that we lack context data regarding these GUCs in
a non-backend context. Why not.

While I was tweaking with the stats code, I got a little bit annoyed
by the timing of the flushes. So one idea I could think about is a
point in a vacuum report, where we could play with a VACUUM command
and some partitions, checking that some relations trigger or not?

But actually, there is even simpler.. As one example, in 051 for
Sawada-san's effective WAL level, we have some local points that could
be made more efficient by targetting only the slot name we want. So
we could use this new facility to make the tests more surgical in the
way the points are run. Perhaps that sounds a bit pedantic, but
something like the attached would be enough for me as a starting
point. We don't really have to be ambitious in the first step, and we
could expand that to tighten as well some of the PID-based checks,
perhaps, for the checkpointer or the startup process. Basic idea is
attached.
--
Michael