Re: [PATCH] Add DNS lookup step to ICAP connect

On 01/07/11 11:53, Alex Rousskov wrote:
> On 06/30/2011 11:11 AM, Tsantilas Christos wrote:
>> And this is the third version.
>>
>> This patch does not include only the DNS lookup for ICAP but other
>> changes too like ICAP connections pool changes, which required to allow
>> ICAP client work.
>
> Why do we need to dynamically allocate theIdleConns? Just curious...
>

The connections pointed to are RefCounted.

>
>> This patch also is not complete. The Max-Connections ICAP feature is
>> currently broken and needs some redesign, to work well.

?! How is it broken? Alex and I just had a major discussion making sure
the open/close conditions were identical now to prior the comm changes.

>
> Sigh. We just got it in!
>
>
>> I think we should move the DNS-lookup and open connection to the ICAP
>> server functionality from Adaptation::Icap::Xaction class to
>> Adaptation::Icap::ServiceRep class.
>
> I am fine with that if you think it is going to make things work again.
> Just be careful when passing the open connection object from Service to
> Xaction. If Xaction is gone, make sure the connection is returned to the
> Service or, at least, closed. IIRC, you already have a custom AsyncCall
> or Dialer that does similar stuff so you can reuse/adjust that code.
>

getConnection() provided by the ServiceRep is currently producing
connections synchronously for Xaction. That will need changing, then it
will be trivial to both get pre-opened connections to Xaction and to
store the list of potential/closed new ones in ServiceRep for use by
other Xaction's. Thus load-balancing across all DNS results.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.12
   Beta testers wanted for 3.2.0.9 and 3.1.12.3