On Tue, Aug 20, 2019 at 6:34 PM Michael Paquier <mich...@paquier.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
