On Wed, Jun 12, 2024 at 02:08:31PM -0400, Robert Haas wrote:
> On Wed, Jun 12, 2024 at 1:54 PM Noah Misch <n...@leadboat.com> wrote:
> > If I were making a list of changes always welcome post-beta, it wouldn't
> > include adding wait event types.  But I don't hesitate to add one if it
> > unblocks a necessary test for a bug present in all versions.
> 
> However, injection points themselves are not present in all versions,
> so even if we invent a new wait-event type, we'll have difficulty
> testing older versions, unless we're planning to back-patch all of
> that infrastructure, which I assume we aren't.

Right.  We could put the injection point tests in v18 only instead of v17+v18.
I feel that would be an overreaction to a dispute about names that show up
only in tests.  Even so, I could accept that.

> Personally, I think the fact that injection point wait events were put
> under Extension is a design mistake that should be corrected before 17
> is out of beta.

Works for me.  I don't personally have a problem with the use of Extension,
since it is a src/test/modules extension creating them.

> > Here's what I'm reading for each person's willingness to tolerate each 
> > option:
> >
> > STRATEGY                        | Paquier | Misch | Haas
> > --------------------------------------------------------
> > new "Injection Point" wait type | maybe   | yes   | yes
> > INJECTION_POINT(...) naming     | yes     | yes   | unknown
> > isolation spec says event names | yes     | no    | unknown
> >
> > Corrections and additional strategy lines welcome.  Robert, how do you judge
> > the lines where I've listed you as "unknown"?
> 
> I'd tolerate INJECTION_POINT() if we had no other option but I think
> it's clearly inferior. Does the last line refer to putting the
> specific wait event names in the isolation spec file? If so, I'd also
> be fine with that.

Yes, the last line does refer to that.  Updated table:

STRATEGY                        | Paquier | Misch | Haas
--------------------------------------------------------
new "Injection Point" wait type | maybe   | yes   | yes
INJECTION_POINT(...) naming     | yes     | yes   | no
isolation spec says event names | yes     | no    | yes

I find that's adequate support for the first line.  If there are no objections
in the next 24hr, I will implement that.


Reply via email to