Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial)

Поиск
Список
Период
Сортировка
От Michael Paquier
Тема Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial)
Дата
Msg-id CAB7nPqRCFWQPp+jqmiZqt_2CUKD2do_XpgStp1rbZm1A=6hYeg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial)  (Fujii Masao <masao.fujii@gmail.com>)
Ответы Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial)
Список pgsql-hackers


On Thu, Jun 4, 2015 at 10:40 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Jun 1, 2015 at 4:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, May 28, 2015 at 9:09 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Since commit de768844, XLogFileCopy of xlog.c returns to caller a
>> pstrdup'd string that can be used afterwards for other things.
>> XLogFileCopy is used in only one place, and it happens that the result
>> string is never freed at all, leaking memory.

That seems to be almost harmless because the startup process will exit
just after calling XLogFileCopy(). No?

Yes that's harmless. My point here is correctness, prevention does not hurt particularly if this code path is used more in the future.

> Also the current error message in case of failure is very C-like:
> elog(ERROR, "InstallXLogFileSegment should not have failed");
> I thought that we to use function names in the error messages.
> Wouldn't something like that be more adapted?
> "could not copy partial log file" or ""could not copy partial log file
> into temporary segment file"

Or we can extend InstallXLogFileSegment so that we can give the log level
to it. When InstallXLogFileSegment is called after XLogFileCopy, we can
give ERROR as the log level and cause an exception to occur if link() or
rename() fails in InstallXLogFileSegment.

That's neat. Done this way in the attached.
--
Michael
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file
Следующее
От: Andrew Dunstan
Дата:
Сообщение: Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file