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.