Re: [DRAFT][MERGE] Cleanup comm outgoing connections in trunk

Alex Rousskov wrote:
> On 05/19/2010 07:05 AM, Amos Jeffries wrote:
>> Henrik Nordström wrote:
>>> tis 2010-05-18 klockan 23:34 +0000 skrev Amos Jeffries:
>>>
>>>> I've discovered the VC connections in DNS will need a re-working to
>>>> handle
>>>> the new TCP connection setup handling. I've left that for now since it
>>>> appears that you are working on redesigning that area anyway. The new
>>>> setup
>>>> routines will play VERY nicely with persistent TCP links to the
>>>> nameservers.
>>> I have not started on the DNS rewrite yet.
>>>
>>>> I took some extra time last night and broke the back of the selection
>>>> and
>>>> forwarding rewrite. I'm now down to the fine detail build errors. When
>>>> those are done I'll push the branch to LP for you to do the DNS fixes on
>>>> top of.
>>> Ok.
>>>
>> Pushed to launchpad: lp:~yadi/squid/cleanup-comm
>
> How can I review the changes as one patch without checking out your branch?
>

Oh. Attached is my latest code.

This has now been run tested and I've been using it for personal
browsing for a short while now no problems.

> Thank you,
>
> Alex.
>
>
>> This builds, but has not yet been run tested.
>>
>> What has changed:
>>
>> ConnectionDetails objects have been renamed Comm::Connection and been
>> extended to hold the FD and Squids' socket flags.
>>
>> Peer selection has been extended to do DNS lookups on the peers chosen
>> for forwarding to and produce a vector<> of possible connection
>> endpoints (squid local IP via tcp_outgoing_address or tproxy) and remote
>> server.
>>
>> Various connection openers have been converted to use the new
>> ConnectStateData API and CommCalls (function based so far).
>>
>>
>> ConnectStateData has been moved into src/comm/ (not yet namespaced) and
>> had all its DNS lookup operations dropped. To be replaced by a looping
>> process of attempting to open a socket and join a link as described by
>> some Comm::Connection or vector<> of same.
>>
>> ConnectStateData::connect() will go away and do some async work. Will
>> come back at some point by calling the handler with COMM_OK,
>> COMM_ERR_CONNECT, COMM_TIMEOUT and ptrs to the Comm::Connection or
>> vector (whichever were passed in).
>> On COMM_OK the Comm::Connection pointer or the first entry of the
>> vector will be an open conn which we can now use.
>> On COMM_ERR_CONNECT the vector will be empty (all tried and
>> discarded), the single ptr will be closed if not NULL.
>> On COMM_TIMEOUT their content is as per COMM_ERR_CONNECT but the vector
>> may have untried paths still present but closed.
>>
>> FD opening, FD problems, connection errors, timeouts, early remote
>> TCP_RST or NACK closure during the setup are all now wrapped out of
>> sight inside ConnectStateData.
>>
>> The main-level component may set FD handlers as needed for read/write
>> and closure of the link in their connection-done handler where the FD
>> first becomes visible to them.
>>
>>
>> Besides the testing there is some work to:
>> * make it obey squid.conf limits on retries and paths looked up.
>> * make DNS TCP links ('VC') work again.

The ref-counting helped a bit here not having to juggle the new/delete
locations. I think it goes, but have not had a conclusive test yet.
Still very inefficient for what it needs to do.

>> * make the CommCalls proper AsynCalls and not function handler based.
>> * make Comm::Connection ref-counted so we can have them stored
>> in the peer details and further reduce the DNS steps.

done.

>> * make ICAP do DNS lookups to set its server Comm::Connection properly.
>> For now it's stuck with the gethostbyname() blocking lookup.
>>
>>
>> Future work once this is stable is to:
>> a) push the IDENT, NAT, EUI and TLS operations down into the Comm layer
>> with simple flags for other layers to turn them on/off as desired.
>> b) make the general code pass Comm::Connection around so everything
>> like ACLs can access the client and server conn when they need to.
>>
>> Amos
>

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.3