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

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...

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

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.

Thank you,

Alex.

> On 06/25/2011 05:04 AM, Amos Jeffries wrote:
>> Version 2. This one resolves a few compile bugs.
>>
>> Ralf: cc'd in case you would like to try testing this.
>>
>> On 25/06/11 03:21, Amos Jeffries wrote:
>>> This seeks to add an async DNS lookup step to the ICAP connection setup
>>> process.
>>>
>>> As a side effect it adds a little bit of tcp_outgoing_address support to
>>> ICAP. Its limited to "dst" ACL at present. So outgoing ACL selections
>>> depending on HTTP request details wont work. Which makes sense since
>>> this connection may be reused for multiple requests.
>>>
>>> I've skipped the idea of using more than one IP result since there seems
>>> to be no sane place to store multiple IPs between connect attempts. It
>>> currently relies on ipcache MarkGood/MarkBad keeping a good usable IP at
>>> the top of the IP set.
>>>
>>
>> Amos
Received on Thu Jun 30 2011 - 23:54:54 MDT