Обсуждение: isolation tester limitation in case of multiple injection points in a single command

On Sun, Oct 19, 2025 at 03:41:00PM +0200, Mihail Nikalayeu wrote:
> Hello, everyone!
>
> While stabilizing tests for [0] I realized I have no idea how to set
> up a pretty simple scenario using an isolation tester.
>
> It looks like this:
> * session S1 start long running query (CREATE INDEX CONCURRENTLY) and
> traps into injection point X
> * session S2 executes some command
> * session S2 wakes up S1
> * now S2 needs to run one more command BUT only AFTER S1 traps into
> another injection point Y (and it is still the same CREATE INDEX)
> <------- tricky part here

Ah, OK.  So you want to have one single command, still wait through
two injection points inside it.  I am wondering about one thing: do
you really require that?  Could it be simpler to have two
permutations, each one of them using one wait point to check your two
scenarios?

> So, my proposal options:
> a) tell me if there's a clear solution I missed
> b) for 'wait' injection_point - learn isolation tester to detect it
> and implement logic like 'S2_query(S1 traps Y)'
> c) learn isolation tester to treat 'notice' from injection_point the
> same way as RAISE NOTICE + (preferable) add possibility to
> apply 'notice' and 'wait' for the same injection point (like
> 'wait+notice')
> d) add injection_point_await - utility to wait until injected point is not
> "waited" by some backend
>
> What do you think? Any other ideas?

Something close to what you are suggested in b), that can be defined
in the specs, sounds like a natural solution to me.  It also feels
like a better approach from the spec's reader perspective, as well:
this would document multiple wait steps in the test itself.
--
Michael

Hello, Michael!

Michael Paquier <michael@paquier.xyz>:
> Ah, OK.  So you want to have one single command, still wait through
> two injection points inside it.  I am wondering about one thing: do
> you really require that?  Could it be simpler to have two
> permutations, each one of them using one wait point to check your two
> scenarios?

Good point, I reviewed all 4 types of permutations - and I was able to
reduce it to 1 of such type.

permutation
    s3_start_create_index(s1_start_upsert, s2_start_upsert)
    s1_start_upsert
    s4_wakeup_define_index_before_set_valid
    s2_start_upsert(s1_start_upsert)
    s4_wakeup_s1_from_invalidate_catalog_snapshot
    s4_wakeup_s2
    s4_wakeup_s1 <---- here

In human words it like this:

permutation
    s3 creates new index and hangs before marking it as valid
    s1 starts UPSERT, caches arbiter indexes and hangs in
InvalidateCatalogSnapshot
    s4 wakes up CREATE INDEX - new index now valid
    concurrent upsert s2 is started and hangs just before insertion
    s4 wakes up s1 - old update reads index definitions, goes throw
check_exclusion_or_unique_constraint and hangs before insertion
    s4 wakes up s2 - concurrent upsert inserts new tuples and finishes
    s4 wakes up s1 - initial upsert going to insert tuple and receives error

So, I have to hang/wake up s1 two times:
* once it cached old arbiters and before it read index definitions
* once it decides it is OK to do insertions

> But AFAIU injection_point's 'notice' is a totally different type of notice
> compared to RAISE NOTICE - so, isolation_tester just ignores it.
I was wrong here - it respects 'notice', I was using it in the wrong order.

This is NOT working (hangs)
permutation
    s1_notice
    s2_noop(s1_notice notices 1)
    s2_next

But this is working:
permutation
    s2_noop(s1_notice notices 1)
    s1_notice
    s2_next

So, it looks like option C ('notice+wait') feels like the simplest
solution for me.

Best regards,
Mikhail.

Hello, Michael!

Currently injection_point.c says [0]:

> /*
> * Allocate and register a new injection point.  A new point should not
> * exist.  For testing purposes this should be fine.
> */

What if we extend that limitation to allow registration of different
types for the same injection point? Like 'wait' and 'notice' (tol be
executed in order they registered).
It feels like a pretty simple solution and covers my needs.

Best regards,
Mikhail.

[0]:
https://github.com/postgres/postgres/blob/7b2eb72b1b8ce4279e42848a3978e781ae239355/src/backend/utils/misc/injection_point.c#L298-L301

On Sun, Nov 09, 2025 at 01:53:00PM +0100, Mihail Nikalayeu wrote:
> What if we extend that limitation to allow registration of different
> types for the same injection point? Like 'wait' and 'notice' (tol be
> executed in order they registered).
> It feels like a pretty simple solution and covers my needs.

It is already possible to achieve that, isn't it?  One can define
side-by-side twice INJECTION_POINT(), then assign a different callback
to each with a strict control of the execution order.
--
Michael

On Sun, Nov 09, 2025 at 11:51:51PM +0100, Mihail Nikalayeu wrote:
> Yes, possible. But the idea is to avoid weird two INJECTION_POINT in a
> row...

FWIW, I think that it's OK to use as style.  The backend-side
injection point implementation is currently quite simple, meaning that
it less prone to future bugs.  For testing purposes, that's an
important property to rely on, IMO.
--
Michael

Hi!

(sorry, I accidentally pressed "reply" instead "reply all) - sent again

Michael Paquier <michael@paquier.xyz> wrote:
>
> FWIW, I think that it's OK to use as style.  The backend-side
> injection point implementation is currently quite simple, meaning that
> it less prone to future bugs.  For testing purposes, that's an
> important property to rely on, IMO.


Just realized that my need is not possible to achieve using multiple
injection points in a row (or different types for the same injection
point) in a race-free way.

This is because I need to receive `notice` BEFORE the backend goes
into `wait` but ONLY AFTER it is guaranteed for WAKE_UP to be
successful.
Yes, the probability of such a race is really low, but still...

So, NOTICE should be sent at [0] while holding spin lock. What do you
think about adding an optional parameter to raise NOTICE with wait?
But it feels weird with the 'notice' type itself...

Any thoughts? Or just ignore that potential low-probability race +
some comments in the test?

Best regards,
Mikhail.

[0]:
https://github.com/postgres/postgres/blob/4eca711bc991954613261b7a314b1e8f5963815c/src/test/modules/injection_points/injection_points.c#L315