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: Alexey Kondratov <a.kondratov@postgrespro.ru>, Alexander Korotkov <a.korotkov@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-09T08:16:22Z
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 Fri, Mar 06, 2020 at 05:22:16PM +0900, Michael Paquier wrote:
> Thanks for the suggestion.  Avoiding dead code makes sense as well
> here.  I'll think about this stuff a bit more first.

Okay, after pondering more on that, here is a first cut with a patch
refactoring restore_command build to src/common/.  One thing I have
changed compared to the other versions is that BuildRestoreCommand()
now returns a boolean to tell if the command was successfully built or
not.

A second thing.  As of now the interface of BuildRestoreCommand()
assumes that the resulting buffer has an allocation of MAXPGPATH.
This should be fine because that's an assumption we rely on a lot in
the code, be it frontend or backend.  However, it could also be a trap
for any caller of this routine if they allocate something smaller.
Wouldn't it be cleaner to pass down the size of the result buffer
directly to BuildRestoreCommand() and use the length given by the
caller of the routine as a sanity check?

Any thoughts?
--
Michael