> On Wed, Nov 7, 2018 at 10:58 AM Alexey Kondratov <a.kondra...@postgrespro.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?