On Tue, Jun 11, 2024 at 01:37:21PM +0900, Michael Paquier wrote: > On Mon, Jun 10, 2024 at 07:19:27PM -0700, Noah Misch wrote: > > On Fri, Jun 07, 2024 at 09:08:03AM -0400, Robert Haas wrote: > >> 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. > > Adding a wait event class for injection point is an interesting > suggestion that would simplify the detection in the isolation function > quite a bit. Are you sure that this is something that would be fit > for v17 material? TBH, I am not sure.
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. > At the end, the test coverage has the highest priority and the bugs > you are addressing are complex enough that isolation tests of this > level are a necessity, so I don't object to what > inplace050-tests-inj-v2.patch introduces with the naming dependency > for the time being on HEAD. I'll just adapt and live with that > depending on what I deal with, while trying to improve HEAD later on. 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'm still wondering if there is something that could be more elegant > than a dedicated class for injection points, but I cannot think about > something that would be better for isolation tests on top of my head. > If there is something I can think of, I'll just go and implement it :) I once considered changing them to use advisory lock waits instead of ConditionVariableSleep(), but I recall that was worse from the perspective of injection points in critical sections.
