Horton

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

Alexey Kondratov <a.kondratov@postgrespro.ru>

View thread
From: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Andrey Borodin <x4mmm@yandex-team.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, vladimirlesk@yandex-team.ru, Dmitriy Sarafannikov <dsarafan@yandex-team.ru>
Date: 2018-11-07T09:58:11Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Move frontend-side archive APIs from src/common/ to src/fe_utils/

  2. Add -c/--restore-target-wal to pg_rewind

  3. Move routine definitions of xlogarchive.c to a new header file

  4. Move routine building restore_command to src/common/

  5. Integrate recovery.conf into postgresql.conf

Attachments

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