On Fri, Jun 07, 2024 at 09:08:03AM -0400, Robert Haas wrote:
> On Thu, Jun 6, 2024 at 7:20 PM Michael Paquier <michael@paquier.xyz> wrote:
> > On Thu, Jun 06, 2024 at 09:48:51AM -0400, Robert Haas wrote:
> > > It's not this patch set's fault, but I'm not very pleased to see that
> > > the injection point wait events have been shoehorned into the
> > > "Extension" category - which they are not - instead of being a new
> > > wait_event_type. That would have avoided the ugly wait-event naming
> > > pattern, inconsistent with everything else, introduced by
> > > inplace050-tests-inj-v1.patch.
> >
> > Not sure to agree with that. The set of core backend APIs supporting
> > injection points have nothing to do with wait events. The library
> > attached to one or more injection points *may* decide to use a wait
> > event like what the wait/wakeup calls in modules/injection_points do,
> > but that's entirely optional. These rely on custom wait events,
> > plugged into the Extension category as the code run is itself in an
> > extension. I am not arguing against the point that it may be
> > interesting to plug in custom wait event categories, but the current
> > design of wait events makes that much harder than what core is
> > currently able to handle, and I am not sure that this brings much at
> > the end as long as the wait event strings can be customized.
> >
> > I've voiced upthread concerns over the naming enforced by the patch
> > and the way it plugs the namings into the isolation functions, by the
> > way.
>
> I think the core code should provide an "Injection Point" wait event
> type and let extensions add specific wait events there, just like you
> did for "Extension".
Michael, could you accept the core code offering that, or not? If so, I am
content to implement that. If not, for injection point wait events, I have
just one priority. The isolation tester already detects lmgr locks without
the test writer teaching it about each lock individually. I want it to have
that same capability for injection points. Do you think we can find something
everyone can accept, having that property? These wait events show up in tests
only, and I'm happy to make the cosmetics be anything compatible with that
detection ability.