[sr-dev] PKG/SHM allocation failure log macros

[Daniel-Constantin Mierla]

Hello,

On 28.12.18 16:37, Victor Seva wrote:
> Hello,
>
> On Fri, 28 Dec 2018 at 16:22, Daniel-Constantin Mierla
> <miconda at gmail.com <mailto:miconda at gmail.com>> wrote:
>
>     I noticed many commits replacing the log messages in case of
>     allocation
>     failure with some macros. That is good, bringing consistency, but I
>     think that we should offer couple of them. The current one is
>     rather dry
>     (meaning that it offers very few context details), which matches
>     most of
>     the existing log messages used in such cases.
>
>     But there are also other log messages for such cases which give more
>     details, like for what the allocation fails, some also giving the
>     requested size of allocation.
>
>     So besides the current two macros (one for shm and one for pkg), we
>     should add few more. Like:
>
>     #define PKG_MEM_ERROR_MSG(m) LM_ERR("could not allocate private memory
>     from pkg pool - %s\n", m);
>
>     So one can do:
>
>     PKG_MEM_ERROR_MSG("needed for htable struct");
>
>     And one to include also the size:
>
>     #define PKG_MEM_ERROR_SZ(s, m) LM_ERR("could not allocate private
>     memory
>     from pkg pool - size: %u - %s\n", (unsigned int)s, m);
>
>     No need to revert what was done, but I think for the future we would
>     preserve better information for troubleshooting in some cases, instead
>     of replacing those messages that now have more details with the bare
>     error log message.
>
>
> I'm not sure about the benefits of adding a custom message but I see
> that having the requested size in the log maybe can be useful in case
> of a mistake in the (re)allocation.
> Do you think that if the pool is exhausted, makes sense to know, with
> a custom log, where the reservation failed?

The custom message is useful to avoid going to source code to figure out
where it fails inside same function. There can be couple of allocations
inside the same function and when one runs an older version and reports
some errors it can happen that the current head of that branche has
different lines for those allocations, so it would require to check out
the exact snapshot. However, sometime that is no longer known if it is
running a build from a tarball that doesn't have the git commit id.
Also, I do not remember if deb/rpm packages put commit id inside the
version string. But I remember I faced couple of situations like these
in the past and decided to add more context in the error message, even
if it was still a static string.

Cheers,
Daniel

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference - May 6-8, 2019 -- www.kamailioworld.com
Kamailio Advanced Training - Mar 4-6, 2019 in Berlin; Mar 25-27, 2019, in Washington, DC, USA -- www.asipto.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20181228/b3d28138/attachment-0001.html>