[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