Re: Cleanup isolation specs from unused steps - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Cleanup isolation specs from unused steps
Date
Msg-id 20190821013424.GC1684@paquier.xyz
Whole thread Raw
In response to Re: Cleanup isolation specs from unused steps  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: Cleanup isolation specs from unused steps  (Melanie Plageman <melanieplageman@gmail.com>)
List pgsql-hackers
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.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Ian Barwick
Date:
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
Next
From: Tatsuo Ishii
Date:
Subject: Re: Serialization questions