Re: ICAP connections under heavy loads

On 06.09.2012 02:30, 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.

It's there to fix an async race condition of Squid vs TCP:

1) the timeout occurs ... timeout AsyncCall is scheduled.
2) the socket TCP completes ... completed AsyncCall is scheduled.
3) timeout call is handled:
   a) if FD is ready use it, terminate the job early as a success (side
effect: cancel the pending completed Call)
   b) else, terminate the job early as a fail.
4) connection completed call is dropped is handled.

The 3a conditional test is what comm_connect_addr() is doing on
timeout. The switch case in ConnOpener::connect() is handling the
multi-state output of the 3a check.

If you can find a cleaner way to rewrite the ConnOpener::connect() and
comm_connect_addr() call chain for the timeout case it should be
duplicated and polished into the timeout() handler.

Amos
Received on Wed Sep 05 2012 - 23:48:46 MDT