Re: /bzr/squid3/trunk/ r11499: Upgrade comm layer Connection handling

On 18/06/11 23:13, Tsantilas Christos wrote:
> Hi all,
> After last commits the ICAP client can not be used, the squid crashes
> when trying to open a connection to the ICAP server.
>
> Looking in the code I found that the problem exist inside the
> Adaptation::Icap::ServiceRep::getConnection method. The new code is not
> exactly equivalent with the old code.
>
> The old code return always a fd to the user. In the case no connection
> found in idle connections, it uses the comm_open to reserve an fd.
>
> The current code, if I am correct, in the case no connection found in
> idle list it just uses a
> connection = new Comm::Connection;
> which just creates a Comm::Connection object with fd=-1. This is causes
> a crash later in Xaction::openConnection.
>
> I believe some code is missing here, but I am not sure if it should be
> added in comm/Connection.* (maybe a second constructor which allocates
> an fd) or in ServiceRep::getConnection function.

Awe heck forgot this one. Sorry.

Comm no longer contains implicit DNS resolution. It needs to be given
the remote IP address before opening.

  Since ServiceRep::getConnection() is strictly synchronous and assumes
that there is only one IP for the service.

Xaction::openConnection() can become async so now contains:
  "TODO: find the IPs and attempt each one if this is a named service"
marks the spot.

  For now it converts IP-addressed services using raw-IP text conversion
"remote= s.cfg().host.termedBuf()". Which is synchronous and
non-blocking but not resolving FQDN.

When FQDN resolution is required do we ...

  a) split openConnection() into two async steps and only use one of the
possibly multiple IPs returned?

OR

  b) lookup the DNS at some far earlier point and store the IPA in
ServiceRep for some sort of load balancing selection of just one by
getConnection()?

Amos

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