Re: [PATCH] ICAP pconn optimization

On 16/06/11 08:07, Alex Rousskov wrote:
> On 06/14/2011 08:43 PM, Amos Jeffries wrote:
>> ICAP services use a "service" model of pconn different from the "TCP
>> destination" model which PconnPool objects are designed for.
>>
>> This patch alters Adaptation::Icap::ServiceRep to use the simpler
>> IdleConnList object for pconn storage. IdleConnList stores a "set of
>> idle connections" more compatible with the ICAP model.
>>
>> In order to implement ICAP max-connections feature the closeN()
>> operation is added to IdleConnList.
>>
>> The result is removal of the complex hash and management operations on
>> push/pop of the idle conn set.
>
>> === modified file 'src/adaptation/icap/ServiceRep.cc'
>> - int connection = theIdleConns.pop(..., retriableXact);
>> + int connection = theIdleConns.findUseableFD();
>
> The pop() method does not allow reuse of connections when the
> transaction cannot be retried. It also prevents the number of idle
> connections from growing uncontrollably when many transactions cannot be
> retried.
>
> As far as I can see, the new ICAP code lost those two critical pieces of
> pconn management logic by calling findUseableFD() directly. Please redo.

PconnPool::pop(..., false) always produces -1. Is exactly equivalent to
closeN(1).

Am I incorrect in thinking that ICAP retriable is equivalent to HTTP
"Connection:close" ? (ie affecting the _end_ of a transaction, not its
beginning)

  That an idle conn, by fact of its existence on the idle list, may be
used by _any_ future transaction?
  Even if that transaction cannot put it back into the idle list?

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