Re: Fix Error Message for allocate_recordbuf() Failure - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Fix Error Message for allocate_recordbuf() Failure
Date
Msg-id CAB7nPqQ6N+pCb7Pkj52x_OVed4JhcsnVDH-QX59fe1R1yT6i9w@mail.gmail.com
Whole thread Raw
In response to Fix Error Message for allocate_recordbuf() Failure  (Shoaib Lari <slari@pivotal.io>)
Responses Re: Fix Error Message for allocate_recordbuf() Failure  (Craig Ringer <craig@2ndquadrant.com>)
Re: Fix Error Message for allocate_recordbuf() Failure  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari <slari@pivotal.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.

And even if we make this thing more verbose, a better approach would
be surely to generate a WARNING message for backends in mcxt.c and
have something printed to stderr for frontends in fe_memutils.c
without calling exit().
-- 
Michael



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Issue with bgworker, SPI and pgstat_report_stat
Next
From: Michael Paquier
Date:
Subject: Re: pgbench - minor fix for meta command only scripts