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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, Liudmila Mantrova <l(dot)mantrova(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, David Steele <david(at)pgmasters(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, vladimirlesk(at)yandex-team(dot)ru, dsarafan(at)yandex-team(dot)ru
Subject: Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line
Date: 2020-03-12 04:39:06
Message-ID: 20200312043906.GC102315@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Move-routine-generating-restore_command-to-src-commo.patch text/x-diff 8.3 KB
0002-Add-c-restore-target-wal-to-pg_rewind.patch text/x-diff 23.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-03-12 04:47:57 Re: Berserk Autovacuum (let's save next Mandrill)
Previous Message Tom Lane 2020-03-12 04:33:21 Re: Refactor compile-time assertion checks for C/C++