On Sat, Sep 19, 2015 at 8:27 AM, Michael Paquier wrote: > On Fri, Sep 18, 2015 at 6:25 PM, Michael Paquier wrote: >> The refactoring of getTimelineHistory as you propose looks like a good >> idea to me, I tried to remove by myself the difference between source >> and target in copy_fetch.c and friends but this gets uglier, >> particularly because of datadir_source in copy_file_range. Not worth >> it. > > Forgot that: > if (ControlFile_target.state != DB_SHUTDOWNED) > pg_fatal("target server must be shut down cleanly\n"); > We may want to allow a target node shutdowned in recovery as well here.
So, attached is a more polished version of this patch, cleaned up of its typos with as well other things. I have noticed for example that it would be more useful to add the debug information of a timeline file fetched from the source or a target server directly in getTimelineHistory. I have as well updated a couple of comments in the code regarding the fact that we do not necessarily use a master as a target node, and mentioned in findCommonAncestorTimeline that we check as well the start position of a timeline to cover the case where both target and source node forked at the same timeline number but with a different WAL fork position. I am marking this patch as ready for committer. It would be cool in the future to use the recovery test suite to have more advanced scenarios tested, but it seems a shame to block this patch because of that.
Thanks a lot. Now patch looks much better.
I found that it doesn't applies cleanly on the current master. Rebased version is attached.
------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company