Re: [Patch] pg_rewind: options to use restore_command fromrecovery.conf or command line

Поиск
Список
Период
Сортировка
От Alexey Kondratov
Тема Re: [Patch] pg_rewind: options to use restore_command fromrecovery.conf or command line
Дата
Msg-id d5f4883dc6ecf5a762e2b8843200ad61@postgrespro.ru
обсуждение исходный текст
Ответ на Re: [Patch] pg_rewind: options to use restore_command fromrecovery.conf or command line  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Ответы Re: [Patch] pg_rewind: options to use restore_command fromrecovery.conf or command line  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Список pgsql-hackers
On 2020-02-26 22:03, Alexander Korotkov wrote:
> On Tue, Feb 25, 2020 at 1:48 PM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
>> 
>> I think usage of chmod() deserves comment.  As I get default
>> permissions are sufficient for work, but we need to set them to
>> satisfy 'check PGDATA permissions' test.
> 
> 
> I've added this comment myself.
> 

Thanks for doing it yourself, I was going to answer tonight, but it 
would be obviously too late.

> 
> I've also fixes some indentation.
> Patch now looks good to me.  I'm going to push it if no objections.
> 

I think that docs should be corrected. Previously Michael was against 
the phrase 'restore_command defined in the postgresql.conf', since it 
also could be defined in any config file included there. We corrected it 
in the pg_rewind --help output, but now docs say:

+        Use the <varname>restore_command</varname> defined in
+        <filename>postgresql.conf</filename> to retrieve WAL files from
+        the WAL archive if these files are no longer available in the
+        <filename>pg_wal</filename> directory of the target cluster.

Probably it should be something like:

+        Use the <varname>restore_command</varname> defined in
+        the target cluster configuration to retrieve WAL files from
+        the WAL archive if these files are no longer available in the
+        <filename>pg_wal</filename> directory.

Here the only text split changed:

-     * Ignore restore_command when not in archive recovery (meaning
-     * we are in crash recovery).
+     * Ignore restore_command when not in archive recovery (meaning we are 
in
+     * crash recovery).

Should we do so in this patch?

I think that this extra dot at the end is not necessary here:

+        pg_log_debug("using config variable restore_command=\'%s\'.", 
restore_command);

If you agree then attached is a patch with all the corrections above. It 
is made with default git format-patch format, but yours were in a 
slightly different format, so I only was able to apply them with git am 
--patch-format=stgit.


--
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
The Russian Postgres Company


Вложения

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

Предыдущее
От: Pavel Stehule
Дата:
Сообщение: Re: proposal: schema variables
Следующее
От: Dave Cramer
Дата:
Сообщение: Re: Error on failed COMMIT