Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)

Поиск
Список
Период
Сортировка
От Michael Paquier
Тема Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)
Дата
Msg-id 20191008025139.GB1613@paquier.xyz
обсуждение исходный текст
Ответ на Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)  (Alexey Kondratov <a.kondratov@postgrespro.ru>)
Список pgsql-hackers
On Mon, Oct 07, 2019 at 03:31:45PM +0300, Alexey Kondratov wrote:
> On 07.10.2019 4:06, Michael Paquier wrote:
>> - --dry-run coverage is basically the same with the local and remote
>> modes, so it seems like a waste of resource to run it for all the
>> tests and all the modes.
>
> My point was to test --dry-run + --write-recover-conf in remote, since the
> last one may cause recovery configuration write without doing any actual
> work, due to some wrong refactoring for example.

Yes, that's possible.  I agree that it would be nice to have an extra
test for that, still I would avoid making that run in all the tests.

>> Patch v3-0002 also had a test to make sure that the source server is
>> shut down cleanly before using it.  I have included that part as
>> well, as the flow feels right.
>>
>> So, Alexey, what do you think?
>
> It looks good for me. Two minor remarks:
>
> +    # option combinations.  As the code paths taken by those tests
> +    # does not change for the "local" and "remote" modes, just run them
>
> I am far from being fluent in English, but should it be 'do not change'
> instead?

That was wrong, fixed.

> +command_fails(
> +    [
> +        'pg_rewind',     '--target-pgdata',
> +        $primary_pgdata, '--source-pgdata',
> +        $standby_pgdata, 'extra_arg1'
> +    ],
>
> Here and below I would prefer traditional options ordering "'--key',
> 'value'". It should be easier to recognizefrom the reader perspective:

While I agree with you, the perl indentation we use has formatted the
code this way.  There is also an argument for keeping it at the end
for clarity (I recall that Windows also requires extra args to be
last when parsing options).  Anyway, I have used a trick by adding
--debug to reach command, which is still useful, so the order of the
options is better at the end.
--
Michael

Вложения

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

Предыдущее
От: "Smith, Peter"
Дата:
Сообщение: RE: Proposal: Make use of C99 designated initialisers fornulls/values arrays
Следующее
От: Noah Misch
Дата:
Сообщение: Re: expressive test macros (was: Report test_atomic_ops() failuresconsistently, via macros)