From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: The return value of allocate_recordbuf() |
Date: | 2015-02-12 07:02:52 |
Message-ID: | CAB7nPqQKg16R-cyKraMYCb7XQKLCwo4oDLHTmD609VbQMfc30A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Feb 11, 2015 at 2:13 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
0001-Add-palloc_extended-for-frontend-and-backend.patch | text/x-patch | 4.7 KB |
0002-Rework-handling-of-OOM-when-allocating-record-buffer.patch | text/x-patch | 2.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2015-02-12 07:26:46 | Re: [REVIEW] Re: Compression of full-page-writes |
Previous Message | Noah Misch | 2015-02-12 05:16:57 | Re: assessing parallel-safety |