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

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)2ndquadrant(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: 2019-01-07 09:12:03
Message-ID: A5DF5C14-AA25-43ED-90E3-5FEB01590B6D@yandex-team.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

Thanks for working on this feature, I believe it solves actual problem of HA systems.

> 30 окт. 2018 г., в 8:01, Michael Paquier <michael(at)paquier(dot)xyz> написал(а):
>
> Another thing I am wondering is: do we actually need something complex?
> What we want to know is what data is necessary to build the file map, so
> we could also add an option to pg_rewind which checks what segments are
> necessary and lets the user know about them?
From my point of view fetching WALs automatically is much better option for automation.

> This also avoids the
> security-related problems of manipulating a command at option-level.
> This kind of options makes folks willing to use more sensitive data on
> command line, which is not always a good idea...

I do not see any new security problems here.. I'd be happy if anyone pointed me out where I can learn about them.

> 26 дек. 2018 г., в 19:11, Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru> написал(а):
>
> Please, find the new version of patch attached.

The refactoring of guc-file looks sane, but I'm not an expert in frontend\backend modularity.

Here are some my notes:
1. RestoreArchivedWAL() shares a lot of code with RestoreArchivedFile(). Is it possible\viable to refactor and extract common part?
2. IMV pg_rewind with %r restore_command should fail. %r is designed to clean archive from WALs, nothing should be deleted in case of fetching WALs for rewind. Last restartpoint has no meaning during rewind. Or does it? If so, let's comment about it.
3. RestoreArchivedFile() checks for signals, is it done by pg_rewind elsewhere?
4. No documentation is updated
5. -R takes precedence over -r without notes. Shouldn't we complain? Or may be we should take one from config, iif nothing found use -R?

Thanks!

Best regards, Andrey Borodin.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2019-01-07 09:17:07 Re: speeding up planning with partitions
Previous Message Masahiko Sawada 2019-01-07 09:10:21 Re: A few new options for vacuumdb