Re: [PATCH] pconn_lifetime

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/09/2014 7:38 p.m., Tsantilas Christos wrote:
> On 09/02/2014 03:51 AM, Amos Jeffries wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>>
>> On 2/09/2014 4:49 a.m., Tsantilas Christos wrote:
>>> Hi all,
>>>
>>> This patch add a new configuration option the 'pconn_lifetime'
>>> to allow users set the desired maximum lifetime of a
>>> persistent connection.
>>>
>>> When set, Squid will close a now-idle persistent connection
>>> that exceeded configured lifetime instead of moving the
>>> connection into the idle connection pool (or equivalent). No
>>> effect on ongoing/active transactions. Connection lifetime is
>>> the time period from the connection acceptance or opening time
>>> until "now".
>>>
>>> This limit is useful in environments with long-lived
>>> connections where Squid configuration or environmental factors
>>> change during a single connection lifetime. If unrestricted,
>>> some connections may last for hours and even days, ignoring
>>> those changes that should have affected their behavior or their
>>> existence.
>>>
>>> This is a Measurement Factory project
>>
>> Two problems with this.
>>
>> * the directive name does not indicate whether it applies to
>> client, server, or ICAP conections.
>
> It applies to any connection, server, client or ICAP connections.

Those are very distinct groups of connections. With very different
lifetime properties.

>
>>
>> * the rationale for its need is a bit screwey. Any directives
>> which affect Squid current running state need to ensure in their
>> post-reconfigure logics that state is consistent. Preferrably
>> create a runner, or use configDoConfigure if that is to much.
>
> Do you mean to run over the persistent connections list and change
> the timeout?

I mean:

1) for the client connections list, set the flag forcing keep-alive to
terminate after current request.
 - only if the ports config changed.

2) For the server pconn idle pools call close() on all existing
comnnections.
 - but why? what condition requires this?

3) for the cache_peer standby pools, calling close()
   - only if the peer config changed.

4) for the ICAP idle pools calling close().
 - but why? what condition needs this other than ICAP config changes,
and thus parent object holding them open being destructed?

5) for the ICAP currently active connections set a flag closing after
current request.
 - but again why? what condition has changed?

>
>>
>> To support that we need some API updates to globally access the
>> currently active connection lists for servers/client/icap. But
>> there are other things like stats and reporting also needing that
>> API, so we should look at adding it instead of yet another
>> "temporary" workaround.
>
> We do not need to make any change to support ICAP and server side
> connections. This is because we are storing this connections to
> IdleConnList or PconnPool structures.
>
> Currently we can not change timeout for client-side keep-alived
> connections. We do not have a list with these connections
> available.
>

That is what I refer to as needed API. Stats reporting and delay pool
reassignnment also need this std::list of active ConnStateData
connections.

Amos
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJUBbfzAAoJELJo5wb/XPRj93IH/i3QbIA/mcU/3EA3mnz2E1Vu
166/k3zlCBAD/NhcD9pFb735VoLSJqIprcfsnCbPoPFKJfX4d4kwSfTtTLshMyBP
vJbyKwcWdSt4ZUybGyeKXXitLaUFkQ8tZQgXzs60msxC58EMAsQ7JB08bEPzV38P
PrIU1YQK/bsH+cSw07CoqiBmLbWYqvSpkVTsPNGXBCTAvPr3gWGz3wNvzZ2c4Ai0
hlVs95XY9WvIIYUjPrRD+BKnBtagdga/1ouclg5mPa7rR6/0txONA7zTXVMAth7c
7jDZh/YY9hEgF9TG9xEwcHsh0w7Wo9lt9l7eZlftdyOGzsZ/ei+FlAy68zfKW7g=
=tLcn
-----END PGP SIGNATURE-----
Received on Tue Sep 02 2014 - 12:28:51 MDT