On 06/08/2015 09:04 PM, Fujii Masao wrote:
On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
<michael.paqu...@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.7080...@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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to