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

Поиск
Список
Период
Сортировка
От Paul Guo
Тема Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)
Дата
Msg-id CAEET0ZGWuweAhLdvp6-svbYSnU5sy7sOO=VmfECenz-S-Yk+ug@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)  (Alexey Kondratov <a.kondratov@postgrespro.ru>)
Ответы Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)  (Alexey Kondratov <a.kondratov@postgrespro.ru>)
Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers

I went through the remaining two patches and they seem to be very clear
and concise. However, there are two points I could complain about:

1) Maybe I've missed it somewhere in the thread above, but currently
pg_rewind allows to run itself with -R and --source-pgdata. In that case
-R option is just swallowed and neither standby.signal, nor
postgresql.auto.conf is written, which is reasonable though. Should it
be stated somehow in the docs that -R option always has to go altogether
with --source-server? Or should pg_rewind notify user that options are
incompatible and no recovery configuration will be written?

I modified code & doc to address this. In code, pg_rewind will error out for the local case.
 
2) Are you going to leave -R option completely without tap-tests?
Attached is a small patch, which tests -R option along with the existing
'remote' case. If needed it may be split into two separate cases. First,
it tests that pg_rewind is able to succeed with minimal permissions
according to the Michael's patch d9f543e [1]. Next, it checks presence
of standby.signal and adds REPLICATION permission to rewind_user to test
that new standby is able to start with generated recovery configuration.

[1]
https://github.com/postgres/postgres/commit/d9f543e9e9be15f92abdeaf870e57ef289020191

 
It seems that we could further disabling recovery info setting code for the 'remote' test case?

-   my $port_standby = $node_standby->port;
-   $node_master->append_conf(
-       'postgresql.conf', qq(
-primary_conninfo='port=$port_standby'
-));
+   if ($test_mode ne "remote")
+   {
+       my $port_standby = $node_standby->port;
+       $node_master->append_conf(
+           'postgresql.conf',
+           qq(primary_conninfo='port=$port_standby'));

-   $node_master->set_standby_mode();
+       $node_master->set_standby_mode();
+   }

Thanks.

Вложения

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

Предыдущее
От: Masahiko Sawada
Дата:
Сообщение: Re: Skip recovery/standby signal files in pg_basebackup
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Skip recovery/standby signal files in pg_basebackup