Horton

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

Andrey Borodin <x4mmm@yandex-team.ru>

View thread
From: Andrey Borodin <x4mmm@yandex-team.ru>
To: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>, Dmitry Dolgov <9erthalion6@gmail.com>, Michael Paquier <michael@paquier.xyz>, Alvaro Herrera <alvherre@2ndquadrant.com>, vladimirlesk@yandex-team.ru, dsarafan@yandex-team.ru
Date: 2019-02-09T11:31:16Z
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

Hi! Thanks for the next version!

> 8 февр. 2019 г., в 18:30, Alexey Kondratov <a.kondratov@postgrespro.ru> написал(а):
> 
> On 21.01.2019 23:50, a.kondratov@postgrespro.ru wrote:
>> Thank you for the review! I have updated the patch according to your
>> comments and remarks. Please, find new version attached.
> 
> During the self-reviewing of the code and tests, I discovered some problems with build on Windows. New version of the patch is attached and it fixes this issue as well as includes some minor code revisions.

I've made one more pass through code and found no important problems.

The patch moves code including these lines
	* XXX this is an unmaintainable crock, because we have to know how to set
	* (or at least what to call to set) every variable that could potentially
	* have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source. However, there's no
	* time to redesign it for 9.1.
But I think it's not the point of this patch to refactor that code.

Here's a typo in postgreslq.conf
+			fprintf(stderr, _("%s: option -r/--use-postgresql-conf is specified, but postgreslq.conf is absent in the target directory\n"),

I'm still not sure guc-file refactoring you made is architecturally correct, I do not feel that my expertise is enough to judge, but everything works.

Besides this, I think you can switch patch to "Ready for committer".

check-world is passing on macbook, docs are here, feature is implemented and tested.

Best regards, Andrey Borodin.