[squid-dev] [PATCH] pconn_lifetime
squid3 at treenet.co.nz
Thu Oct 2 05:24:31 UTC 2014
-----BEGIN PGP SIGNED MESSAGE-----
On 2/10/2014 5:23 a.m., Tsantilas Christos wrote:
> So which is the decision? Can the patch go to trunk? Should the
> patch replaced with on other solution?
We already have client_lifetime directive which is documented as doing
exactly what this new directive does.
However, what that directive *actually* does is very different from
the documented behaviour - it is also used for controlling
between-request idle timeout (breaking client_idle_pconn_timeout by
making it 1 week!) and also server request timout (pretending to be a
timeout for duration between sending request and starting to read reply).
I think extending it to a better solution for the above bugs and
1) In the existing client_side_reply.cc keep-alive conditions there
exists a line:
else if (http->getConn() && http->getConn()->port->listenConn == NULL)
your patch retains it as:
if (http->getConn()->port->listenConn == NULL)
- this check is *supposed* to be the check for when, as you put it
"environments with long-lived connections where Squid configuration
... change during a single connection lifetime."
- that is buggy. A check of !Comm::IsConnOpen() is what will
determine a live/dead listener socket.
2) There is more work to do to integrate it properly with the existing
pconn timeout and lifetime directives.
I request that we include these in this patch:
* change ConnStateData::clientParseRequests() to use
conn->timeLeft(Config.Timeout.clientIdlePconn) when setting timeout
- this is another bug in the existing code that is retained by your
patch and needs fixing.
* change your new directive "pconn_lifetime" to be
"client_pconn_lifetime" since it is specifically scoped to client
- keeping or updating the client_lifetime documentation.
* add some new directive (or find existing one that supposed to be
applied) for the server waiting-for-reply timout period.
- use that instead of Config.Timeout.lifetime in src/http.cc
* 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.
NP: at this point Config.Timeout.lifetime should not be used anywhere
in Squid and can be dropped.
* check the SSL code where Config.Timeout.lifetime is mentioned in
comment to see if the timeout there is still relevant. Probably is but
needs checking and the comment updating.
Optional / future work:
A) this depends on having a matching/symmetrical server_pconn_lifetime
directive for the server connections.
Since there would be separate lifetimes for each 'side', I think we
should followup with a patch making a Comm::Connection::startTime_ be
initialized by Comm::TcpAcceptor() and Comm::ConnOpener() with the
Then timeLeft() method does not need dependency on SquidConfig.h and
connection lifetimes will actually be predictable across a
reconfigure. At present a reconfigure changing lifetime will change
some connections but not others depending on whether they actually
receive a request between the reconfigure and one of the two old/new
B) after the above we can also de-duplicate the timeLeft(X) calls to
one inside commSetConnTimeout().
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)
-----END PGP SIGNATURE-----
More information about the squid-dev