[squid-dev] [PATCH] pconn_lifetime for squid-3.5.x

2015-05-25 Thread Tsantilas Christos
I am attaching a patch which implements the pconn_lifetime feature for 
squid-3.5.x, in the case someone want to use it or we decide that it can 
be inlcuded in squid-3.5.x.


This patch include the recent fixes for pconn_lifetime feature.

This is a Measurement Factory project

Regards,
   Christos
Add pconn_lifetime to limit 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
=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h	2015-02-18 08:50:00 +
+++ src/SquidConfig.h	2015-03-22 10:52:21 +
@@ -72,40 +72,41 @@
 #if USE_HTTP_VIOLATIONS
 time_t negativeTtl;
 #endif
 time_t maxStale;
 time_t negativeDnsTtl;
 time_t positiveDnsTtl;
 time_t shutdownLifetime;
 time_t backgroundPingRate;
 
 struct {
 time_t read;
 time_t write;
 time_t lifetime;
 time_t connect;
 time_t forward;
 time_t peer_connect;
 time_t request;
 time_t clientIdlePconn;
 time_t serverIdlePconn;
 time_t ftpClientIdle;
+time_t pconnLifetime; /// pconn_lifetime in squid.conf
 time_t siteSelect;
 time_t deadPeer;
 int icp_query;  /* msec */
 int icp_query_max;  /* msec */
 int icp_query_min;  /* msec */
 int mcast_icp_query;/* msec */
 time_msec_t idns_retransmit;
 time_msec_t idns_query;
 } Timeout;
 size_t maxRequestHeaderSize;
 int64_t maxRequestBodySize;
 size_t maxRequestBufferSize;
 size_t maxReplyHeaderSize;
 AclSizeLimit *ReplyBodySize;
 
 struct {
 unsigned short icp;
 #if USE_HTCP
 
 unsigned short htcp;

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2015-02-18 10:30:07 +
+++ src/cf.data.pre	2015-03-22 10:54:02 +
@@ -6027,40 +6027,65 @@
 TYPE: time_t
 LOC: Config.Timeout.lifetime
 DEFAULT: 1 day
 DOC_START
 	The maximum amount of time a client (browser) is allowed to
 	remain connected to the cache process.  This protects the Cache
 	from having a lot of sockets (and hence file descriptors) tied up
 	in a CLOSE_WAIT state from remote clients that go away without
 	properly shutting down (either because of a network failure or
 	because of a poor client implementation).  The default is one
 	day, 1440 minutes.
 
 	NOTE:  The default value is intended to be much larger than any
 	client would ever need to be connected to your cache.  You
 	should probably change client_lifetime only as a last resort.
 	If you seem to have many client connections tying up
 	filedescriptors, we recommend first tuning the read_timeout,
 	request_timeout, persistent_request_timeout and quick_abort values.
 DOC_END
 
+NAME: pconn_lifetime
+COMMENT: time-units
+TYPE: time_t
+LOC: Config.Timeout.pconnLifetime
+DEFAULT: 0 seconds
+DOC_START
+	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.
+	 
+	Currently, a new lifetime value supplied via Squid reconfiguration
+	has no effect on already idle connections unless they become busy.
+	 
+	When set to '0' this limit is not used.
+DOC_END
+
 NAME: half_closed_clients
 TYPE: onoff
 LOC: Config.onoff.half_closed_clients
 DEFAULT: off
 DOC_START
 	Some clients may shutdown the sending side of their TCP
 	connections, while leaving their receiving sides open.	Sometimes,
 	Squid can not tell the difference between a half-closed and a
 	fully-closed TCP connection.
 
 	By default, Squid will immediately close client connections when
 	read(2) returns no more data to read.
 
 	Change this option to 'on' and Squid will keep open connections
 	until a read(2) or write(2) on the socket returns an error.
 	This may show some benefits for reverse proxies. But if not
 	it is 

Re: [squid-dev] [PATCH] pconn_lifetime robustness fixes

2015-04-28 Thread Tsantilas Christos

Patch applied to trunk as r14046.


On 04/27/2015 06:40 PM, Tsantilas Christos wrote:

If there is not any objection I will apply this patch to trunk.


On 04/15/2015 07:11 PM, Tsantilas Christos wrote:

Hi all,
  I am attaching which fixes pconn_lifetime feature.
We had a long discussion for this feature, which is resulted to the
patch r13780, but unfortunately, Measurement Factory customers reported
problems:

1. Squid closed connections with partially received requests when they
reached pconn_lifetime limit. We should only close _idle_ connections.

2. When connecting to Squid without sending anything for longer than
pconn_lifetime, the connection hangs if the request is sent after the
waiting period.

3. The connection also hangs if the initial request is starting to be
transmitted but then there is a longer pause before the request is
completed.

Please read the patch preamble for more informations.

This is a Measurement Factory project.


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev



___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] pconn_lifetime robustness fixes

2015-04-27 Thread Tsantilas Christos

If there is not any objection I will apply this patch to trunk.


On 04/15/2015 07:11 PM, Tsantilas Christos wrote:

Hi all,
  I am attaching which fixes pconn_lifetime feature.
We had a long discussion for this feature, which is resulted to the
patch r13780, but unfortunately, Measurement Factory customers reported
problems:

1. Squid closed connections with partially received requests when they
reached pconn_lifetime limit. We should only close _idle_ connections.

2. When connecting to Squid without sending anything for longer than
pconn_lifetime, the connection hangs if the request is sent after the
waiting period.

3. The connection also hangs if the initial request is starting to be
transmitted but then there is a longer pause before the request is
completed.

Please read the patch preamble for more informations.

This is a Measurement Factory project.


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev



___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


[squid-dev] [PATCH] pconn_lifetime robustness fixes

2015-04-15 Thread Tsantilas Christos

Hi all,
 I am attaching which fixes pconn_lifetime feature.
We had a long discussion for this feature, which is resulted to the 
patch r13780, but unfortunately, Measurement Factory customers reported 
problems:


1. Squid closed connections with partially received requests when they 
reached pconn_lifetime limit. We should only close _idle_ connections.


2. When connecting to Squid without sending anything for longer than 
pconn_lifetime, the connection hangs if the request is sent after the 
waiting period.


3. The connection also hangs if the initial request is starting to be 
transmitted but then there is a longer pause before the request is 
completed.


Please read the patch preamble for more informations.

This is a Measurement Factory project.
pconn_lifetime robustness fixes

This patch changes pconn_lifetime (r13780) to abort only really idle 
persistent connections (when they timeout). It removes some extra features
(added to pconn_lifetime during the feature review) because they break things
when aggressive timeouts is combined with picky clients. Specifically,

1. Squid closed connections with partially received requests when they
   reached pconn_lifetime limit. We should only close _idle_ connections.

2. When connecting to Squid without sending anything for longer than
   pconn_lifetime, the connection hangs if the request is sent after the
   waiting period.

3. The connection also hangs if the initial request is starting to be
   transmitted but then there is a longer pause before the request is
   completed.

Most of the above problems are easy to trigger only when using very aggressive
pconn_lifetime settings that the feature was not designed for, but they still
can be considered bugs from admins point of view. Fixes:

* Do not stop reading a partially received request when we are timing out,
  to avoid aborting that request.

* Do not set keepalive flag based on the pconn_lifetime timeout. We cannot
  predict whether some new request data is going to be read (and reset the
  idle timeout clock) before our Connection:close response is sent back.

HTTP clients are supposed to recover from such races, but some apparently
do not, especially if it is their first request on the connection.

This is a Measurement Factory project.

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2015-03-16 09:52:13 +
+++ src/client_side.cc	2015-03-23 09:53:55 +
@@ -3045,46 +3045,40 @@
  */
 bool
 ConnStateData::clientParseRequests()
 {
 bool parsed_req = false;
 
 debugs(33, 5, HERE  clientConnection  : attempting to parse);
 
 // Loop while we have read bytes that are not needed for producing the body
 // On errors, bodyPipe may become nil, but readMore will be cleared
 while (!in.buf.isEmpty()  !bodyPipe  flags.readMore) {
 
 /* Don't try to parse if the buffer is empty */
 if (in.buf.isEmpty())
 break;
 
 /* Limit the number of concurrent requests */
 if (concurrentRequestQueueFilled())
 break;
 
-/*Do not read more requests if persistent connection lifetime exceeded*/
-if (Config.Timeout.pconnLifetime  clientConnection-lifeTime()  Config.Timeout.pconnLifetime) {
-flags.readMore = false;
-break;
-}
-
 // try to parse the PROXY protocol header magic bytes
 if (needProxyProtocolHeader_  !parseProxyProtocolHeader())
 break;
 
 if (ClientSocketContext *context = parseOneRequest()) {
 debugs(33, 5, clientConnection  : done parsing a request);
 
 AsyncCall::Pointer timeoutCall = commCbCall(5, 4, clientLifetimeTimeout,
  CommTimeoutCbPtrFun(clientLifetimeTimeout, context-http));
 commSetConnTimeout(clientConnection, Config.Timeout.lifetime, timeoutCall);
 
 context-registerWithConn();
 
 processParsedRequest(context);
 
 parsed_req = true; // XXX: do we really need to parse everything right NOW ?
 
 if (context-mayUseConnection()) {
 debugs(33, 3, HERE  Not parsing new requests, as this request may need the connection);
 break;

=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc	2015-01-13 07:25:36 +
+++ src/client_side_reply.cc	2015-03-23 09:59:02 +
@@ -1487,44 +1487,40 @@
 } else if (reply-bodySize(request-method)  0  !maySendChunkedReply) {
 debugs(88, 3, clientBuildReplyHeader: can't keep-alive, unknown body size );
 request-flags.proxyKeepalive = false;
 } else if (fdUsageHigh() !request-flags.mustKeepalive) {
 debugs(88, 3, clientBuildReplyHeader: Not many unused FDs, can't keep-alive);
 request-flags.proxyKeepalive = false;
 } else if (request-flags.sslBumped  !reply-persistent()) {
 // We do not really have to close, but we pretend we are a tunnel.
 debugs(88, 3, 

Re: [squid-dev] [PATCH] pconn_lifetime

2014-12-24 Thread Tsantilas Christos

Patch applied to trunk (revno: 13780).

On 12/23/2014 08:52 PM, Tsantilas Christos wrote:

If there is not any objection I will apply this patch to trunk.

On 12/15/2014 12:39 PM, Tsantilas Christos wrote:

Hi all,

  I am attaching a new patch for the pconn_lifetime feature. A first
patch has posted in mailing list and discussed under the mail thread
with the same title 1-2 months ago.

This patch is similar to the old one posted, with a small fix to better
handle pipelined connections:
   1. finish interpreting the Nth request
  check whether pconn_lifetime has expired
   2. if pconn_lifetime has expired, then stop further reading and
  do not interpret any already read raw bytes of the N+1st request
   3. otherwise, read and interpret read raw bytes of the N+1st request
  and go to #1.

The above should be enough. The pipelined requests are always
idempotent, they do not have body data to take care about, and the web
clients knows that if a pipelined HTTP request failed, it should be
retried in a new connection.

I must recall the following about this patch:
- The pconn_lifetime it applies to any persistent connection,
server, client, or ICAP.
- This patch does not fix other problems may exist in current squid.
- The pconn_lifetime should not confused with the client_lifetime
timeout. They have different purpose.

This is a Measurement Factory project



___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] pconn_lifetime

2014-12-23 Thread Tsantilas Christos

If there is not any objection I will apply this patch to trunk.

On 12/15/2014 12:39 PM, Tsantilas Christos wrote:

Hi all,

  I am attaching a new patch for the pconn_lifetime feature. A first
patch has posted in mailing list and discussed under the mail thread
with the same title 1-2 months ago.

This patch is similar to the old one posted, with a small fix to better
handle pipelined connections:
   1. finish interpreting the Nth request
  check whether pconn_lifetime has expired
   2. if pconn_lifetime has expired, then stop further reading and
  do not interpret any already read raw bytes of the N+1st request
   3. otherwise, read and interpret read raw bytes of the N+1st request
  and go to #1.

The above should be enough. The pipelined requests are always
idempotent, they do not have body data to take care about, and the web
clients knows that if a pipelined HTTP request failed, it should be
retried in a new connection.

I must recall the following about this patch:
- The pconn_lifetime it applies to any persistent connection,
server, client, or ICAP.
- This patch does not fix other problems may exist in current squid.
- The pconn_lifetime should not confused with the client_lifetime
timeout. They have different purpose.

This is a Measurement Factory project



___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev



___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] pconn_lifetime

2014-12-15 Thread Tsantilas Christos

Hi all,

 I am attaching a new patch for the pconn_lifetime feature. A first 
patch has posted in mailing list and discussed under the mail thread 
with the same title 1-2 months ago.


This patch is similar to the old one posted, with a small fix to better 
handle pipelined connections:

  1. finish interpreting the Nth request
 check whether pconn_lifetime has expired
  2. if pconn_lifetime has expired, then stop further reading and
 do not interpret any already read raw bytes of the N+1st request
  3. otherwise, read and interpret read raw bytes of the N+1st request
 and go to #1.

The above should be enough. The pipelined requests are always 
idempotent, they do not have body data to take care about, and the web 
clients knows that if a pipelined HTTP request failed, it should be 
retried in a new connection.


I must recall the following about this patch:
   - The pconn_lifetime it applies to any persistent connection, 
server, client, or ICAP.

   - This patch does not fix other problems may exist in current squid.
   - The pconn_lifetime should not confused with the client_lifetime 
timeout. They have different purpose.


This is a Measurement Factory project

pconn_lifetime

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 option has the following behaviour when pipelined requests tunneled
to a connection where its lifetime expired:

 1. finish interpreting the Nth request
check whether pconn_lifetime has expired
 2. if pconn_lifetime has expired, then stop further reading and
do not interpret any already read raw bytes of the N+1st request
 3. otherwise, read and interpret read raw bytes of the N+1st request
and go to #1.


This is a Measurement Factory project
=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h	2014-12-04 14:00:17 +
+++ src/SquidConfig.h	2014-12-15 09:28:15 +
@@ -72,40 +72,41 @@
 #if USE_HTTP_VIOLATIONS
 time_t negativeTtl;
 #endif
 time_t maxStale;
 time_t negativeDnsTtl;
 time_t positiveDnsTtl;
 time_t shutdownLifetime;
 time_t backgroundPingRate;
 
 struct {
 time_t read;
 time_t write;
 time_t lifetime;
 time_t connect;
 time_t forward;
 time_t peer_connect;
 time_t request;
 time_t clientIdlePconn;
 time_t serverIdlePconn;
 time_t ftpClientIdle;
+time_t pconnLifetime; /// pconn_lifetime in squid.conf
 time_t siteSelect;
 time_t deadPeer;
 int icp_query;  /* msec */
 int icp_query_max;  /* msec */
 int icp_query_min;  /* msec */
 int mcast_icp_query;/* msec */
 time_msec_t idns_retransmit;
 time_msec_t idns_query;
 time_t urlRewrite;
 } Timeout;
 size_t maxRequestHeaderSize;
 int64_t maxRequestBodySize;
 int64_t maxChunkedRequestBodySize;
 size_t maxRequestBufferSize;
 size_t maxReplyHeaderSize;
 AclSizeLimit *ReplyBodySize;
 
 struct {
 unsigned short icp;
 #if USE_HTCP

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2014-12-08 11:25:58 +
+++ src/cf.data.pre	2014-12-15 09:28:15 +
@@ -6129,40 +6129,65 @@
 TYPE: time_t
 LOC: Config.Timeout.lifetime
 DEFAULT: 1 day
 DOC_START
 	The maximum amount of time a client (browser) is allowed to
 	remain connected to the cache process.  This protects the Cache
 	from having a lot of sockets (and hence file descriptors) tied up
 	in a CLOSE_WAIT state from remote clients that go away without
 	properly shutting down (either because of a network failure or
 	because of a poor client implementation).  The default is one
 	day, 1440 minutes.
 
 	NOTE:  The default value is intended to be much larger than any
 	client would ever need to be connected to your cache.  You
 	should probably change client_lifetime only as a last resort.
 	If you seem to have many client connections tying up
 	filedescriptors, we recommend first tuning the read_timeout,
 	request_timeout, persistent_request_timeout and quick_abort values.
 DOC_END
 
+NAME: pconn_lifetime
+COMMENT: time-units
+TYPE: time_t
+LOC: Config.Timeout.pconnLifetime
+DEFAULT: 0 seconds
+DOC_START
+	Desired maximum lifetime of a persistent connection.
+	When set, Squid will close a now-idle 

Re: [squid-dev] [PATCH] pconn_lifetime

2014-10-03 Thread Alex Rousskov
On 10/03/2014 11:57 AM, Amos Jeffries wrote:
 On 4/10/2014 3:52 a.m., Alex Rousskov wrote:
 On 10/01/2014 11:24 PM, Amos Jeffries wrote:
 
 We already have client_lifetime directive which is documented
 as doing exactly what this new directive does. 
 (http://www.squid-cache.org/Doc/config/client_lifetime/).
 
 No, client_lifetime is not documented (and is not meant) to do
 what we want pconn_lifetime to do. Client_lifetime is supposed to
 kill client connections exceeding the configured lifetime,
 _regardless_ of the client (and client connection) state. We do
 not want that for pconn_lifetime which only affects _idle_
 connections.
 
 Lets have a look at that documentation then...
 
 http://www.squid-cache.org/Doc/config/client_lifetime/ :  The
 maximum amount of time a client (browser) is allowed to remain
 connected to the cache process. 
 
 - scope: client (browser)

Yes.


 And Christos patch description:  the desired maximum lifetime of a
 persistent connection. 

Yes. No side limitation here.


 Having read through the patch. What it does is *only* set a timer 
 running when the client connection is accepted, and limits (most) 
 timeouts applied later on that connection to be no more than the 
 configured total.

The patch should limit the _total_ pconn lifetime (but enforce that
limit only when Squid thinks the persistent connection is [going to
be] idle). Also, it should apply the same logic to origin server
connections that we pool. AFAICT, the patch does all of the above. If
it does not, there is a bug in the patch. More on that further below.

Please note that I use the term idle from Squid point of view as in
may be closed without causing much harm. That is, it is an opposite
of in the middle of handling some transaction. We could split hairs
arguing about some corner cases, but I hope we do not have to do that.


 Moreover, pconn_lifetime is meant to apply to all connections,
 not just the connections from a client.
 
 Then Christos patch is seriously incomplete, because it only
 affects connections arriving from clients. The lifetime is never
 set on any of the other types of connection that make up all.

AFAICT, the patch affects both HTTP user and origin connections. The
origin connections are affected by pconn.cc changes. Perhaps I
misinterpreted that change or you missed it?

I am not 100% sure, but I suspect those pconn.cc changes affect ICAP
connection as well.

As we discussed, the patch does not add new code to pool idle HTTP
user connections -- that big change is outside this patch scope, but
when/if that code is added, the same approach can be applied there
trivially.


 * update tunnel.cc client connection timeouts to use 
 conn-timeLeft(). - maybe with some new tunnel-specific
 lifetime config item. But I think your new config lifetime is
 appropriate for now at least.
 
 There is no concept of idle persistent connection for TCP
 tunnels so we should not apply pconn_lifetime to them right now.
 
 You added idle to that description. The code in question *does
 not* limit itself to idle connections, nor persistent connections.

I did not add that. The proposed feature should apply to idle
connections only, as documented: When set, Squid will close a
now-idle persistent connection that ...


 So do we at least have a consensus on terminology:
 
 That lifetime for a connection means from its opening 
 accept()/connect() to its close()  ??

Yes, as documented in the patch: 'Connection lifetime is the time
period from the connection acceptance or opening time until now'.


 That the bits named FOO_timeout means some duration of that
 lifetime spent doing a FOO ??

Timeout and lifetime are very different things so I would not mix them
up like that, but yes, agents may do FOO during their lifetime (often
many times!).


Hope this clarifies,

Alex.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [PATCH] pconn_lifetime

2014-09-02 Thread Tsantilas Christos

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.



* 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?




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.





Amos





Re: [PATCH] pconn_lifetime

2014-09-02 Thread Amos Jeffries
-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-


Re: [PATCH] pconn_lifetime

2014-09-02 Thread Alex Rousskov
On 09/02/2014 06:28 AM, Amos Jeffries wrote:
 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.

Those properties are about the same as far as this option is
concerned. It would be possible to split this option into 4 or more
different ones, to allow for more precise control, but I think that
should not be done until there is a specific use case for such a split.


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

The alternative you suggest is bit screwy on two counts:

1) It does not address environmental factors outside of Squid
knowledge. A trivial example would be typical IP table rules that
allow already accepted connections. If the acceptance criteria change,
the admin may want the already accepted connections to be nicely
closed to make sure they do not violate the new rules.

2) Given the number of squid.conf directives with side-effects (and
side-effects of side-effects), it is unlikely that a reasonable effort
would be sufficient to identify all of them. Moreover, in some cases,
it would probably be very difficult to judge whether there is a side
effect that warrants connection closure. While I agree that we should
improve reconfigure handling in this area (and have posted specific
proposals for that), I do not think what you are suggesting is a
practical alternative for the proposed new directive in the
foreseeable future.


 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.

Port configuration adjustments is only one of the many changes the new
option is meant to address. Moreover, correctly determining what port
configuration changes should lead to automatic pconn closures is very
difficult!


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

Unknown. The option is for admins that know what those conditions are.


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

Same as (1) above.


 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?

Same as (2) above.


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

Same as (2) above.



 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.

I agree that having an API to iterate idle user pconns would be
useful. Implementing that correctly is not easy because those pconns
are owned by other objects. Do you insist that the API is developed as
a precondition for this patch acceptance, despite the facts that it it
is not 

[PATCH] pconn_lifetime

2014-09-01 Thread Tsantilas Christos

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



pconn_lifetime

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
=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h	2014-08-26 09:01:27 +
+++ src/SquidConfig.h	2014-09-01 10:30:40 +
@@ -92,40 +92,41 @@
 #if USE_HTTP_VIOLATIONS
 time_t negativeTtl;
 #endif
 time_t maxStale;
 time_t negativeDnsTtl;
 time_t positiveDnsTtl;
 time_t shutdownLifetime;
 time_t backgroundPingRate;
 
 struct {
 time_t read;
 time_t write;
 time_t lifetime;
 time_t connect;
 time_t forward;
 time_t peer_connect;
 time_t request;
 time_t clientIdlePconn;
 time_t serverIdlePconn;
 time_t ftpClientIdle;
+time_t pconnLifetime; /// pconn_lifetime in squid.conf
 time_t siteSelect;
 time_t deadPeer;
 int icp_query;  /* msec */
 int icp_query_max;  /* msec */
 int icp_query_min;  /* msec */
 int mcast_icp_query;/* msec */
 time_msec_t idns_retransmit;
 time_msec_t idns_query;
 } Timeout;
 size_t maxRequestHeaderSize;
 int64_t maxRequestBodySize;
 int64_t maxChunkedRequestBodySize;
 size_t maxRequestBufferSize;
 size_t maxReplyHeaderSize;
 AclSizeLimit *ReplyBodySize;
 
 struct {
 unsigned short icp;
 #if USE_HTCP
 

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2014-08-26 09:01:27 +
+++ src/cf.data.pre	2014-09-01 15:13:28 +
@@ -5981,40 +5981,65 @@
 TYPE: time_t
 LOC: Config.Timeout.lifetime
 DEFAULT: 1 day
 DOC_START
 	The maximum amount of time a client (browser) is allowed to
 	remain connected to the cache process.  This protects the Cache
 	from having a lot of sockets (and hence file descriptors) tied up
 	in a CLOSE_WAIT state from remote clients that go away without
 	properly shutting down (either because of a network failure or
 	because of a poor client implementation).  The default is one
 	day, 1440 minutes.
 
 	NOTE:  The default value is intended to be much larger than any
 	client would ever need to be connected to your cache.  You
 	should probably change client_lifetime only as a last resort.
 	If you seem to have many client connections tying up
 	filedescriptors, we recommend first tuning the read_timeout,
 	request_timeout, persistent_request_timeout and quick_abort values.
 DOC_END
 
+NAME: pconn_lifetime
+COMMENT: time-units
+TYPE: time_t
+LOC: Config.Timeout.pconnLifetime
+DEFAULT: 0 seconds
+DOC_START
+	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.
+	
+	Currently, a new lifetime value supplied via Squid reconfiguration
+	has no effect on already idle connections unless they become busy.
+	
+	When set to '0' this limit is not used.
+DOC_END
+
 NAME: 

Re: [PATCH] pconn_lifetime

2014-09-01 Thread Amos Jeffries
-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.

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

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.

Amos

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

iQEcBAEBAgAGBQJUBRSPAAoJELJo5wb/XPRj+wQH/A/XVulpmlHL0aIFJBGJyL6m
GrB15odh+bK2U3K2++ZK+B3VDhNs6X44sG/XdA+bxGSw5E01oeIXQEZPoxzlAzO9
YBJx8zpBtnSlUAwuTwptomZPmbfsWDfOjtQQ0KLlccBpHJnb89M1w9O6bV3XT0hz
CRMPW1tY50R6U/HEBJWeXwwdeRfXJmeFru7sHE/evZCre17eVQ79zrcnlVjMciTD
Ag1ctW7C5u87b8iec81aW4X3dMG2IoQSER97dZ2Cde+tVqArK0UlfC9qZIi99DrT
7/WmdZpzPZyTJrCtZw8LD0iIMur7ez1o7kvSs4Z5AP0EPKAvHaTWwK9oASl2QDE=
=WtX2
-END PGP SIGNATURE-