Re: The return value of allocate_recordbuf() - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: The return value of allocate_recordbuf()
Date
Msg-id CAB7nPqQKg16R-cyKraMYCb7XQKLCwo4oDLHTmD609VbQMfc30A@mail.gmail.com
Whole thread Raw
In response to Re: The return value of allocate_recordbuf()  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: The return value of allocate_recordbuf()  (Bruce Momjian <bruce@momjian.us>)
Re: The return value of allocate_recordbuf()  (Fujii Masao <masao.fujii@gmail.com>)
List pgsql-hackers
On Wed, Feb 11, 2015 at 2:13 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> MemoryContextAllocExtended() was added, so isn't it time to replace palloc()
>> with MemoryContextAllocExtended(CurrentMemoryContext, MCXT_ALLOC_NO_OOM)
>> in allocate_recordbuf()?
>
> Yeah, let's move on here, but not with this patch I am afraid as this
> breaks any client utility using xlogreader.c in frontend, like
> pg_xlogdump or pg_rewind because MemoryContextAllocExtended is not
> available in frontend, only in backend. We are going to need something
> like palloc_noerror, defined on both backend (mcxt.c) and frontend
> (fe_memutils.c) side.

Unfortunately, that gets us back into the exact terminological dispute
that we were hoping to avoid.  Perhaps we could compromise on
palloc_extended().

Yes, why not using palloc_extended instead of palloc_noerror that has been clearly rejected in the other thread. Now, for palloc_extended we should copy the flags of MemoryContextAllocExtended to fe_memutils.h and have the same interface between frontend and backend (note that MCXT_ALLOC_HUGE has no real utility for frontends). Attached is a patch to achieve that, completed with a second patch for the problem related to this thread. Note that in the second patch I have added an ERROR in logical.c after calling XLogReaderAllocate, this was missing..

> Btw, the huge flag should be used as well as palloc only allows
> allocation up to 1GB, and this is incompatible with ~9.4 where malloc
> is used.

I think that flag should be used only if it's needed, not just on the
grounds that 9.4 has no such limit.  In general, limiting allocations
to 1GB has been good to us; for example, it prevents a corrupted TOAST
length from causing a memory allocation large enough to crash the
machine (yes, there are machines you can crash with a giant memory
allocation!).  We should bypass that limit only where it is clearly
necessary.

Fine for me to error out in this code path if we try more than 1GB of allocation.
Regards,
--
Michael
Attachment

pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: assessing parallel-safety
Next
From: Michael Paquier
Date:
Subject: Re: [REVIEW] Re: Compression of full-page-writes