Horton

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

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>, Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Alexey Kondratov <a.kondratov@postgrespro.ru>, Liudmila Mantrova <l.mantrova@postgrespro.ru>, Thomas Munro <thomas.munro@gmail.com>, Andrey Borodin <x4mmm@yandex-team.ru>, David Steele <david@pgmasters.net>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Dmitry Dolgov <9erthalion6@gmail.com>, vladimirlesk@yandex-team.ru, dsarafan@yandex-team.ru
Date: 2020-03-12T04:39:06Z
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 Wed, Mar 11, 2020 at 12:27:03PM +0900, Michael Paquier wrote:
>> Also, I think the function comment could stand some more detailing.
> 
> What kind of additional information would you like to add on top of
> what the updated version attached does?

I have been working more on that, and attached is an updated version
of the main feature based on 0001, attached as 0002.  I have applied
the following changes, fixing some actual bugs while on it:
- Added back expectedSize, so as frontends can also fetch things other
than WAL segments from the archives, including history files.
pg_rewind needs only WAL segments, still other tools may want more.
- Fixed an issue with ENOENT when doing stat() on a segment after
running the restore command.  Like the backend, there is an argument
for looping here.
- Switched the several failures to issue exit() for the several
failure types possible instead of looping.  If a file has an incorrect
expected size or that stat() fails for a non-ENOENT, things are more
likely going to repeat if the frontend tool loops, so exiting
immediately is safer.

I think that we need a better name for RestoreArchivedFile() in
fe_archive.c.  The backend uses RestoreArchivedFile(), and the
previous versions of the patch switched to RestoreArchivedWALFile()
which is incorrect if working on something else than a WAL segment.
The version attached uses FrontendRestoreArchivedFile(), still we
could do better.

I'd like to commit the refactoring piece in 0001 tomorrow, then let's
move on with the rest as of 0002.  If more comments and docs are
needed for archive.c, let's continue discussing that.
--
Michael