Re: [Patch] pg_rewind: options to use restore_command fromrecovery.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@paquier.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.kondratov@postgrespro.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
commonpart? 
2. IMV pg_rewind with %r restore_command should fail. %r is designed to clean archive from WALs, nothing should be
deletedin case of fetching WALs for rewind. Last restartpoint has no meaning during rewind. Or does it? If so, let's
commentabout 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
nothingfound use -R? 

Thanks!

Best regards, Andrey Borodin.