From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial) |
Date: | 2015-06-09 03:47:49 |
Message-ID: | CAHGQGwHbUuzq8LNVVevLRLdTzgMYXZHCN0FS-Asiv-+USDy6YQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jun 9, 2015 at 6:25 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> On 06/08/2015 09:04 PM, Fujii Masao wrote:
>>
>> On Mon, Jun 8, 2015 at 11:52 AM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)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.
I'm not sure how much it's really nicer to keep the refactoring,
but I agree that it's better to make the code look same to make
the back-patching easier.
Regards,
--
Fujii Masao
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2015-06-09 03:55:14 | Aggregate Supporting Functions |
Previous Message | Fujii Masao | 2015-06-09 03:39:41 | Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file |