From: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Shoaib Lari <slari(at)pivotal(dot)io>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fix Error Message for allocate_recordbuf() Failure |
Date: | 2016-07-11 05:58:22 |
Message-ID: | CAMsr+YHTu7oGLG7YTfuep6Z_EG3_fT+7W0sF+4Zh=guB1z+AFw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11 July 2016 at 12:04, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari <slari(at)pivotal(dot)io> wrote:
> > Besides making the error message more informative, we had to modify
> > allocate_recordbuf() to return the actual number of bytes that were being
> > allocated.
>
> - report_invalid_record(state, "record length %u at %X/%X too long",
> - total_len,
> - (uint32) (RecPtr >> 32), (uint32) RecPtr);
> + report_invalid_record(state,
> + "cannot allocate %u bytes for record
> length %u at %X/%X",
> + newSizeOut, total_len, (uint32) (RecPtr >>
> 32),
> + (uint32) RecPtr);
>
> It does not look like a good idea to me to complicate the interface of
> allocate_recordbuf just to make more verbose one error message,
> meaning that it basically a complain related to the fact that
> palloc_extended(MCXT_ALLOC_NO_OOM) does not mention to the user the
> size that it has tried to allocate before returning NULL. Do you have
> a use case that would prove to be useful if this extra piece of
> information is provided? Because it seems to me that we don't really
> care if we know how much memory it has failed to allocate, we only
> want to know that it *has* failed and take actions only based on that.
>
I actually find details of how much memory we tried to allocate to be
really useful in other places that do emit it. Sometimes it's been an
important clue when trying to figure out what's going on on a remote system
with no or limited access. Even if it just tells me "we were probably
incrementally allocating lots of small values since this failure is small"
vs "this allocation is huge, look for something unusually large" or "this
allocation is impossibly huge / some suspicious value look for memory
corruption".
I tend to agree with your suggestion about a better approach but do think
emitting details on allocation failures is useful. At least when people
turn VM overcommit off ...
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2016-07-11 07:40:11 | Re: [BUG] pg_basebackup from disconnected standby fails |
Previous Message | Fabien COELHO | 2016-07-11 05:30:14 | Re: pgbench - minor doc improvements |