Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-03 Thread Fujii Masao
On Fri, Apr 3, 2015 at 2:30 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Apr 3, 2015 at 12:56 PM, Fujii Masao wrote: The first patch looks good to me basically. But I have one comment: shouldn't we expose pg_malloc_extended as a global function like we did pg_malloc? Some

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-03 Thread Michael Paquier
On Fri, Apr 3, 2015 at 6:35 PM, Fujii Masao wrote: Regarding the second patch, you added the checks of the return value of XLogReaderAllocate(). But it seems half-baked. XLogReaderAllocate() still uses palloc(), but don't we need to replace it with palloc_extended(), too? Doh, you are right. I

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-03 Thread Fujii Masao
On Fri, Apr 3, 2015 at 8:37 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Apr 3, 2015 at 6:35 PM, Fujii Masao wrote: Regarding the second patch, you added the checks of the return value of XLogReaderAllocate(). But it seems half-baked. XLogReaderAllocate() still uses palloc(),

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-03 Thread Michael Paquier
On Fri, Apr 3, 2015 at 9:57 PM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Apr 3, 2015 at 8:37 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Apr 3, 2015 at 6:35 PM, Fujii Masao wrote: Regarding the second patch, you added the checks of the return value of

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-02 Thread Michael Paquier
On Fri, Apr 3, 2015 at 12:56 PM, Fujii Masao wrote: The first patch looks good to me basically. But I have one comment: shouldn't we expose pg_malloc_extended as a global function like we did pg_malloc? Some frontends might need to use it in the future. Yes, it makes sense as the other

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-02 Thread Fujii Masao
On Thu, Feb 12, 2015 at 4:02 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Feb 11, 2015 at 2:13 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-01 Thread Bruce Momjian
On Thu, Feb 12, 2015 at 04:02:52PM +0900, Michael Paquier wrote: 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

Re: [HACKERS] The return value of allocate_recordbuf()

2015-04-01 Thread Michael Paquier
On Thu, Apr 2, 2015 at 9:10 AM, Bruce Momjian br...@momjian.us wrote: Where are we on this? If we want to have allocate_recordbuf error out properly on frontend side, we are going to need a equivalent of MemoryContextAllocExtended for frontends in the shape of palloc_extended able to take

Re: [HACKERS] The return value of allocate_recordbuf()

2015-02-11 Thread Michael Paquier
On Wed, Feb 11, 2015 at 2:13 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao masao.fu...@gmail.com wrote: MemoryContextAllocExtended() was added, so isn't it time to

Re: [HACKERS] The return value of allocate_recordbuf()

2015-02-10 Thread Robert Haas
On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao masao.fu...@gmail.com wrote: MemoryContextAllocExtended() was added, so isn't it time to replace palloc() with MemoryContextAllocExtended(CurrentMemoryContext,

Re: [HACKERS] The return value of allocate_recordbuf()

2015-02-09 Thread Michael Paquier
On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao masao.fu...@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

Re: [HACKERS] The return value of allocate_recordbuf()

2015-02-09 Thread Fujii Masao
On Mon, Jan 5, 2015 at 1:25 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Jan 1, 2015 at 1:10 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Dec 29, 2014 at 6:14 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. There is no way to check beforehand if a palloc()

Re: [HACKERS] The return value of allocate_recordbuf()

2015-01-08 Thread Andres Freund
Hi, On 2015-01-05 14:18:35 +0900, Michael Paquier wrote: Note that for 9.4, I think that we should complain about an OOM in logical.c where malloc is used as now process would simply crash if NULL is returned by XLogReaderAllocate. That's the object of the second patch. Yes, that's clearly

Re: [HACKERS] The return value of allocate_recordbuf()

2015-01-04 Thread Michael Paquier
On Mon, Dec 29, 2014 at 8:14 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 12/26/2014 09:31 AM, Fujii Masao wrote: Hi, While reviewing FPW compression patch, I found that allocate_recordbuf() always returns TRUE though its source code comment says that FALSE is returned if out of

Re: [HACKERS] The return value of allocate_recordbuf()

2015-01-04 Thread Michael Paquier
On Thu, Jan 1, 2015 at 1:10 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Dec 29, 2014 at 6:14 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. There is no way to check beforehand if a palloc() will fail because of OOM. We could check for MaxAllocSize, though. I think we

Re: [HACKERS] The return value of allocate_recordbuf()

2014-12-31 Thread Robert Haas
On Mon, Dec 29, 2014 at 6:14 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. There is no way to check beforehand if a palloc() will fail because of OOM. We could check for MaxAllocSize, though. I think we need a version of palloc that returns NULL instead of throwing an error. The

Re: [HACKERS] The return value of allocate_recordbuf()

2014-12-29 Thread Heikki Linnakangas
On 12/26/2014 09:31 AM, Fujii Masao wrote: Hi, While reviewing FPW compression patch, I found that allocate_recordbuf() always returns TRUE though its source code comment says that FALSE is returned if out of memory. Its return value is checked in two places, but which is clearly useless.