[squid-dev] [PATCH] shared_memory_locking

[Alex Rousskov]

On 03/10/2016 08:28 PM, Amos Jeffries wrote:

>>> * fatalf() sends a FATAL level error to cache.log. No need to preceed it
>>> with a less important debugs ERROR message saying the same thing.

>> There is such a need, unfortunately: The FATAL error is printed much
>> later than the error is found, making triage a lot more difficult in
>> some cases. In the extreme cases, the FATAL error is not printed at all
>> (because Squid crashes long before fatal.cc code gets to printing the
>> error). Fixing this fatal() problem is outside this patch scope.


> I'm talking about these lines:
> 
>  ... {
> +        const int savedError = errno;
> +        debugs(54, DBG_IMPORTANT, "ERROR: shared_memory_locking enabled
> but mlock(" << theName << ',' << theSize << ") failed: " <<
> xstrerr(savedError));
> +        fatalf("shared_memory_locking on but failed to mlock(%s, %"
> PRId64 "): %s\n",
> +               theName.termedBuf(), theSize, xstrerr(savedError));
> +    }

Yes, me too.


>> To partially address your concern, I set debugs() level to 5 and removed
>> the ERROR prefix. This still helps with triage when higher debugging
>> levels are configured and makes the new code consistent with most other
>> fatal() calls in Segment.cc.

> If there is a bug in fatalf() that's something else that needs to be
> fixed. 

Agreed, but, as I said, fixing fatalf() is out of scope.


> But there has been no sign that I'm aware of about any such issue
> in for any of the hundreds of other calls to it. Please dont make bad
> code here just for that.

It looks like I should not have wasted time explaining why this debugs()
is needed and why it actually improves code consistency in Segment.cc,
but I am not going to fight over a debugs() line. The patch without it
is attached.

Alex.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: shm-lock-t5.patch
Type: text/x-diff
Size: 11367 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20160310/0364c0fa/attachment.patch>