Re: ICAP connections under heavy loads

On 09/05/2012 09:27 AM, Alexander Komyagin wrote:

> So you think that it's ok for comm_coonect_addr() to return COMM_OK if
> it was called before the appropriate select() notification. Am I right?

Hard to say for sure since comm_coonect_addr() lacks an API description,
and there are at least three similar but different ways the function is
being used by Squid.

One natural way to define this API is to say that it should return
COMM_OK if and only if the socket is connected (making any select
notifications irrelevant). However, this definition may be too
CPU-expensive and/or too unportable to support. And this level of
certainty may not actually be needed for current Squid needs!

I would not be surprised if there is some gray area where we cannot
really tell for sure (without too much additional overhead or
portability risk) whether the async socket is connected. The Stevens
book seem to imply that much. Inside that gray area, the function
should probably return COMM_OK so that the rest of the code works: If we
guessed wrong, we will get failures during I/O, but the code has to deal
with those anyway.

In other words, if you want to work on this, consider defining the API
based on current Squid needs and then make sure we support those minimum
requirements, keeping overheads and portability in mind. However, I
would _start_ by fixing the calling code first (as it may affect the
minimum requirements) -- your ConnOpener patch was a step in that direction.

HTH,

Alex.

> On Wed, 2012-09-05 at 08:30 -0600, Alex Rousskov wrote:
>> On 09/05/2012 03:32 AM, Alexander Komyagin wrote:
>>> On Tue, 2012-09-04 at 09:16 -0600, Alex Rousskov wrote:
>>
>>>> Again, I hope that this trick is not needed to solve your problem, and I
>>>> am worried that it will cause more/different problems elsewhere. I would
>>>> recommend fixing CommOpener instead. If _that_ is not sufficient, we can
>>>> discuss appropriate low-level enhancements.
>>>
>>> Both things shall be fixed, IMHO.
>>
>> If double-connect is not needed, we should not introduce it. AFAICT,
>> double-connect is an attempt to cope with a bug in higher-level code. We
>> should fix that bug and see if that is sufficient. If it is not
>> sufficient, we should evaluate why and only then add appropriate
>> low-level code if needed.
>>
>> The primary goal here is to fix the underlying issue, not just to find a
>> workaround (which you have already provided).
>>
>>
>> Thank you,
>>
>> Alex.
>>
>
Received on Wed Sep 05 2012 - 16:00:07 MDT