From: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
---|---|
To: | a(dot)kondratov(at)postgrespro(dot)ru |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, x4mmm(at)yandex-team(dot)ru, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, 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: | 2018-11-30 16:04:01 |
Message-ID: | CA+q6zcUw9KaV-L3khD9axOHU8x+sxota_pQP8-LqqFiOtgQmTA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Wed, Nov 7, 2018 at 10:58 AM Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru> wrote:
>
> On 30.10.2018 06:01, Michael Paquier wrote:
>
> > On Mon, Oct 29, 2018 at 12:09:21PM +0300, Alexey Kondratov wrote:
> >> Currently in the patch, with dry-run option (-n) pg_rewind only fetches
> >> missing WALs to be able to build file map, while doesn't touch any data
> >> files. So I guess it behaves exactly as you described and we do not need a
> >> separate tool.
> > Makes sense perhaps. Fetching only WAL segments which are needed for
> > the file map is critical, as you don't want to spend bandwidth for
> > nothing. Now, I look at your patch, and I can see things to complain
> > about, at least three at short glance:
> > - The TAP test added will fail on Windows.
>
> Thank you for this. Build on Windows has been broken as well. I fixed it
> in the new version of patch, please find attached.
Just to confirm, patch still can be applied without conflicts, and pass all the
tests. Also I like the original motivation for the feature, sounds pretty
useful. For now I'm moving it to the next CF.
> > - Simply copy-pasting RestoreArchivedWAL() from the backend code to
> > pg_rewind is not an acceptable option. You don't care about %r either
> > in this case.
>
> According to the docs [1] %r is a valid alias and may be used in
> restore_command too, so if we take restore_command from recovery.conf it
> might be there. If we just drop it, then restore_command may stop
> working. Though I do not know real life examples of restore_command with
> %r, we should treat it in expected way (as backend does), of course if
> we want an option to take it from recovery.conf.
>
> > - Reusing the GUC parser is something I would avoid as well. Not worth
> > the complexity.
>
> Yes, I don't like it either. I will try to make guc-file.l frontend safe.
Any success with that?
From | Date | Subject | |
---|---|---|---|
Next Message | Dmitry Dolgov | 2018-11-30 16:13:44 | Re: POC: Cleaning up orphaned files using undo logs |
Previous Message | Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?= | 2018-11-30 15:44:38 | Re: [PATCH] Improve tab completion for CREATE TABLE |