Re: Issue with markers in isolation tester? Or not?

On Tue, Jan 14, 2025 at 12:16:22PM +0100, Michail Nikolaev wrote:
> --- a/src/test/isolation/README
> +++ b/src/test/isolation/README
> @@ -183,9 +183,9 @@ A marker consisting solely of a step name indicates that this step may
> not be reported as completing until that other step has completed.
> This allows stabilizing cases where two queries might be seen to complete
> in either order. Note that this step can be *launched* before the other
> -step has completed. (If the other step is used more than once in the
> -current permutation, this step cannot complete while any of those
> -instances is active.)
> +step has completed or launched. (If the other step is used more than once
> +in the current permutation, this step cannot complete while any of those
> +instances are not yet complete.)

I misunderstood, and I was mistaken to see this as a bug fix. The
isolationtester is acting per its definition, and this would be a definition
change. Do others have opinions on the merits of today's definition vs. the
proposed definition?

I'd need to review the motivating test to form my own opinion on whether the
new definition makes it easier to write tests. I'm optimistic that it does
make things easier, because I think one can get the old behavior by defining
two steps with the same commands. Thanks for the patch. You could bundle
this in the thread that wants this to stabilize that thread's CI results.

As a side note on the broader topic of the difficulty of stabilizing isolation
test race conditions, I've thought it would be helpful to have a "strict
blocking condition mode" that complains at the end of the test run if the spec
had completion order ambiguities. I think these are the rules sufficient to
have a fully-specified completion order:

1. Every step that waits needs a blocking condition.
2. No two steps A & B name the same step C as a blocking condition. Instead,
make B block on A.
3. If a step appears in another step's blocking condition and is not the last
step listed in the permutation, the step listed after the unblocker needs a
blocking condition on a formerly-blocked step.

> +session s1
> +step after { SELECT injection_points_run('injection-points-wait-1'); }

FYI, I prefer the convention of ending each step name with the session number,
i.e. s/after/after1/ here. The first isolation specs used that style. I
mildly prefer it over the newer alternative styles, and I strongly prefer
styles that convey the session identity over styles that don't. No need to
send a new patch version, though.