Re: Cleanup isolation specs from unused steps

On Tue, Aug 20, 2019 at 6:34 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Tue, Aug 20, 2019 at 09:54:56AM -0400, Alvaro Herrera wrote:
> > On 2019-Aug-20, Tom Lane wrote:
> >> If you can warn in both cases, that'd be OK perhaps. But Alvaro's
> >> description of the intended use of dry-run makes it sound like
> >> it would be expected for there to be unreferenced steps, since there'd
> >> be no permutations yet in the input.
>
> v2 of the patch warns of any unused steps in dry-run mode. If no
> permutations are defined in the input spec, all steps get used
> (actually that's not a factorial as the per-session ordering is
> preserved), so I would expect no warnings to be generated and the
> patch does that. If the test includes some permutations, then I would
> expect dry-run to complain about the steps which are defined in the
> spec but not used. The patch also does that. Do you see a problem
> with that?
>
> > Well, Heikki/Kevin's original intention was that if no permutations are
> > specified, then all possible permutations are generated internally and
> > the spec is run with that. The intended use of dry-run was to do just
> > that (generate all possible permutations) and print that list, so that
> > it could be trimmed down by the test author. In this mode of operation,
> > all steps are always used, so there'd be no warning printed. However,
> > when a test file has a largish number of steps then the list is, um, a
> > bit long. Before the deadlock-test hacking, you could run with such a
> > list anyway and any permutations that caused a blockage would be
> > reported right away as an invalid permutation -- quick enough.
> > Currently it sleeps for absurdly long on those cases, so this is no
> > longer feasible.
> >
> > This is why I say that the current dry-run mode could be removed with no
> > loss of useful functionality.
>
> Hmm. Even if one does not do something deadlock-specific, the list
> printed could still be useful, no? This for example works now that I
> look at it:
> ./isolationtester -n < specs/my_spec.spec
>
> I am wondering if we should not actually keep dry_run, but rename it
> to something like --print-permutations to print the set of
> permutations which would be run as part of the spec, and also have an
> option which is able to print out all permutations possible, like
> --print-all-permutations. Simply ripping out the mode would be fine
> by me as it does not seem to be used, but keeping it around does not
> induce really much extra maintenance cost.
>

So, I think I completely misunderstood the purpose of 'dry-run'. If no
one is using it, having a check for unused steps in dry-run may not be
useful.

+1 to renaming it to --print-permutations and, potentially,
adding --print-all-permutations

--
Melanie Plageman