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

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, vladimirlesk(at)yandex-team(dot)ru, Dmitriy Sarafannikov <dsarafan(at)yandex-team(dot)ru>
Subject: Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line
Date: 2018-11-07 09:58:11
Message-ID: 3e432374-d21b-9ab7-06e0-a0785e5e3fd0@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 30.10.2018 06:01, Michael Paquier wrote:

> On Mon, Oct 29, 2018 at 12:09:21PM +0300, Alexey Kondratov wrote:
>> Currently in the patch, with dry-run option (-n) pg_rewind only fetches
>> missing WALs to be able to build file map, while doesn't touch any data
>> files. So I guess it behaves exactly as you described and we do not need a
>> separate tool.
> Makes sense perhaps. Fetching only WAL segments which are needed for
> the file map is critical, as you don't want to spend bandwidth for
> nothing. Now, I look at your patch, and I can see things to complain
> about, at least three at short glance:
> - The TAP test added will fail on Windows.

Thank you for this. Build on Windows has been broken as well. I fixed it
in the new version of patch, please find attached.

> - Simply copy-pasting RestoreArchivedWAL() from the backend code to
> pg_rewind is not an acceptable option. You don't care about %r either
> in this case.

According to the docs [1] %r is a valid alias and may be used in
restore_command too, so if we take restore_command from recovery.conf it
might be there. If we just drop it, then restore_command may stop
working. Though I do not know real life examples of restore_command with
%r, we should treat it in expected way (as backend does), of course if
we want an option to take it from recovery.conf.

> - Reusing the GUC parser is something I would avoid as well. Not worth
> the complexity.

Yes, I don't like it either. I will try to make guc-file.l frontend safe.

[1] https://www.postgresql.org/docs/11/archive-recovery-settings.html

--
Alexey Kondratov

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

Attachment Content-Type Size
pg_rewind-restore_command-v1.1.patch text/x-patch 38.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2018-11-07 10:03:32 Re: BUG #15449: file_fdw using program cause exit code error when using LIMIT
Previous Message Fabien COELHO 2018-11-07 09:01:20 Re: csv format for psql