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

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.