Horton

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

Alexander Korotkov <a.korotkov@postgrespro.ru>

View thread
From: Alexander Korotkov <a.korotkov@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.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-10T10:05:40Z
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

On Tue, Mar 10, 2020 at 7:28 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Mar 09, 2020 at 03:38:29PM +0530, Kuntal Ghosh wrote:
> > That's a good suggestion. But, it's unlikely that a caller would pass
> > something longer than MAXPGPATH and we indeed use that value a lot in
> > the code. IMHO, it looks okay to me to have that assumption here as
> > well.
>
> Well, a more serious problem would be to allocate something smaller
> than MAXPGPATH.  This reminds me a bit of 09ec55b9 where we did not
> correctly design from the start the base64 encode and decode routines
> for SCRAM, so I'd rather design this one correctly from the start as
> per the attached.  Alexey, Alexander, what do you think?

Two options seem reasonable to me in this case.  The first is to pass
length as additional argument as you did.  The second option is to
make argument a pointer to fixed-size array as following.

extern bool BuildRestoreCommand(const char *restoreCommand,
                                const char *xlogpath,   /* %p */
                                const char *xlogfname,  /* %f */
                                const char *lastRestartPointFname,  /* %r */
                                char (*commandResult)[MAXPGPATH]);

Passing pointer to array of different size would cause an error.  The
downside of this approach is that passing palloc'd chunk of memory as
commandResult would be less convenient.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company