Re: Cleanup isolation specs from unused steps

Поиск
Список
Период
Сортировка
От Melanie Plageman
Тема Re: Cleanup isolation specs from unused steps
Дата
Msg-id CAAKRu_bLJuEqk=e4z-sHbyj3f+Rddshp7LDriZ3Aewdn+abCAg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Cleanup isolation specs from unused steps  (Michael Paquier <michael@paquier.xyz>)
Ответы Re: Cleanup isolation specs from unused steps  (Michael Paquier <michael@paquier.xyz>)
Список pgsql-hackers

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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Melanie Plageman
Дата:
Сообщение: Re: Cleanup isolation specs from unused steps
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: Cleanup isolation specs from unused steps