On 06/08/2015 09:04 PM, Fujii Masao wrote:
> On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Fri, Jun 5, 2015 at 10:45 PM, Fujii Masao wrote:
>>> Why don't we call InstallXLogFileSegment() at the end of XLogFileCopy()?
>>> If we do that, the risk of memory leak you're worried will disappear at all.
>>
>> Yes, that looks fine, XLogFileCopy() would copy to a temporary file,
>> then install it definitely. Updated patch attached.
>
> Thanks for updating the patch! Looks good to me. Applied.
XLogFileCopy() used to call InstallXLogFileSegment(), until I refactored
that in commit de7688442f5aaa03da60416a6aa3474738718803. That commit
added another caller of XLogFileCopy(), which didn't want to install the
segment. However, I later partially reverted that patch in commit
7cbee7c0a1db668c60c020a3fd1e3234daa562a9, and those changes to
XLogFileCopy() were not really needed anymore. I decided not to revert
those changes at that point because that refactoring seemed sensible
anyway. See my email at
http://www.postgresql.org/message-id/555DD101.7080209@iki.fi about that.
I'm still not sure if I should've just reverted that refactoring, to
make XLogFileCopy() look the same in master and back-branches, which
makes back-patching easier, or keep the refactoring, because it makes
the code slightly nicer. But the current situation is the worst of both
worlds: the interface of XLogFileCopy() is no better than it used to be,
but it's different enough to cause merge conflicts. At this point, it's
probably best to revert the code to look the same as in 9.4.
- Heikki
