RE: Injection point: fix race condition between attach and cleanup

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Michael Paquier' <michael(at)paquier(dot)xyz>
Cc: "'pgsql-hackers(at)lists(dot)postgresql(dot)org'" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>
Subject: RE: Injection point: fix race condition between attach and cleanup
Date: 2026-11-13 09:43:17
Message-ID: OS7PR01MB1831729457F5C9E0E727D224DF57F2@OS7PR01MB18317.jpnprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael,

> Yeah, sure. Or just make sure that the first backend has exited
> before attaching something else to the same point. Or just use a
> differently-named point for the second backend, in the same path.
> There are a couple of options.

You meant everything can be handled on the test code side, right? Per our finding,
010_index_concurrently_upsert can fail due to this issue, and alternative fix
patch for it was to ensure background sessions were exit. See "nocfbot" patch in [1].
Remained other patches tries to fix the root cause for future developers.
Sawada-san suggested the approach in the original thread [2], he have not seen
the implementation yet though.

> FWIW, I've read your patch, and I am absolutely unconvinced about the
> fact that we need to complicate the server side injpoint functionality
> in core for only this purpose. Test facilities like this one should
> remain simple in design, even if they require tweaks sometimes.
> Complicating them would lead to bugs in the facility. I know that I
> am pretty adamant about this part of the code, but injpoints are used
> for many cases across the whole tree, and what you are doing is
> changing one property for another, complicating the code even further,
> while on it.

Hmm, thank you for the feedback. My alternative idea was to match the MyProcPid
and InjectionPointCondition::pid, but it was also complex. InjectionPointDetach()
is implemented in the core, but the condition is controlled in the extension side
thus it's opaque from the core.

Do some others have simpler ideas? Otherwise, we might choose fixing the test code
to prevent the future BF failure.

[1]: https://www.postgresql.org/message-id/flat/avVNVbUeLngeqiEM%40paquier.xyz#157b2774b851c26371f5e4efc8d9827b
[2]: https://www.postgresql.org/message-id/CAD21AoDbakvmxM2Ggn-80VCNej_0f2-_0E0teyMnPN_3T3YWQw%40mail.gmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andreas Karlsson 2026-11-13 09:53:56 Re: [PATCH] pg_input_is_valid() reuses the first type name inside a PL/pgSQL loop
Previous Message Peter Eisentraut 2026-11-13 09:36:29 Re: -Wcast-qual home stretch