Followed your suggestions in [t4] patch.
Eduard.
2016-03-07 17:20 GMT+03:00 Amos Jeffries <[email protected]>:
> * To fit with the rest of the directives this should be called
> server_pconn_for_nonretriable.
>
> * This is not a RFC violation, so you dont need to wrap any of its code
> in USE_HTTP_VIOLATIONS.
> The current Squid behaviour was just a hack Alex added years back to
> prevent the old behaviour this is trying to selectively re-enable now.
> Since it was causing so many client-visible error messages when we first
> moved to HTTP/1.1 defaults.
>
> * What the risk is should probaly be noted in the documentation; That
> the requests it enables to re-use pconn can result in 500 errors
> reaching clients/users eyes if there are any network delivery issues, or
> the server closes its connection while the request is still bufferd
by TCP.
>
> * only indent cf.data.pre text with 1 tab, not 2.
Added a new ACL-driven server_pconn_for_nonretriable option.
This option provides fine-grained control over persistent connection
reuse when forwarding HTTP requests that Squid cannot retry. It is useful
in environments where opening new connections is very expensive
(e.g., all connections are secured with TLS with complex client and server
certificate validation) and race conditions associated with persistent
connections are very rare and/or only cause minor problems
Example:
acl SpeedIsWorthTheRisk method POST
server_pconn_for_nonretriable allow SpeedIsWorthTheRisk
--- src/FwdState.cc 2016-02-13 05:44:58 +0000
+++ src/FwdState.cc 2016-03-08 12:18:43 +0000
@@ -560,68 +560,61 @@
}
if (entry->store_status != STORE_PENDING)
return false;
if (!entry->isEmpty())
return false;
if (n_tries > Config.forward_max_tries)
return false;
if (squid_curtime - start_t > Config.Timeout.forward)
return false;
if (flags.dont_retry)
return false;
if (request->bodyNibbled())
return false;
// NP: not yet actually connected anywhere. retry is safe.
if (!flags.connected_okay)
return true;
if (!checkRetriable())
return false;
return true;
}
-/*
- * FwdState::checkRetriable
- *
- * Return TRUE if this is the kind of request that can be retried
- * after a failure. If the request is not retriable then we don't
- * want to risk sending it on a persistent connection. Instead we'll
- * force it to go on a new HTTP connection.
- */
+/// Whether we may try sending this request again after a failure.
bool
FwdState::checkRetriable()
{
// Optimize: A compliant proxy may retry PUTs, but Squid lacks the [rather
// complicated] code required to protect the PUT request body from being
// nibbled during the first try. Thus, Squid cannot retry some PUTs today.
if (request->body_pipe != NULL)
return false;
// RFC2616 9.1 Safe and Idempotent Methods
return (request->method.isHttpSafe() || request->method.isIdempotent());
}
void
FwdState::serverClosed(int fd)
{
// XXX: fd is often -1 here
debugs(17, 2, "FD " << fd << " " << entry->url() << " after " <<
(fd >= 0 ? fd_table[fd].pconn.uses : -1) << " requests");
if (fd >= 0 && serverConnection()->fd == fd)
fwdPconnPool->noteUses(fd_table[fd].pconn.uses);
retryOrBail();
}
void
FwdState::retryOrBail()
{
if (checkRetry()) {
debugs(17, 3, HERE << "re-forwarding (" << n_tries << " tries, " << (squid_curtime - start_t) << " secs)");
// we should retry the same destination if it failed due to pconn race
@@ -1158,63 +1151,68 @@
case Http::scServiceUnavailable:
return Config.retry.onerror;
default:
return false;
}
/* NOTREACHED */
}
/**
* Decide where details need to be gathered to correctly describe a persistent connection.
* What is needed:
* - the address/port details about this link
* - domain name of server at other end of this link (either peer or requested host)
*/
void
FwdState::pconnPush(Comm::ConnectionPointer &conn, const char *domain)
{
if (conn->getPeer()) {
fwdPconnPool->push(conn, NULL);
} else {
fwdPconnPool->push(conn, domain);
}
}
Comm::ConnectionPointer
FwdState::pconnPop(const Comm::ConnectionPointer &dest, const char *domain)
{
+ bool retriable = checkRetriable();
+ if (!retriable && Config.accessList.serverPconnForNonretriable) {
+ ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request, NULL);
+ retriable = (ch.fastCheck() == ACCESS_ALLOWED);
+ }
// always call shared pool first because we need to close an idle
// connection there if we have to use a standby connection.
- Comm::ConnectionPointer conn = fwdPconnPool->pop(dest, domain, checkRetriable());
+ Comm::ConnectionPointer conn = fwdPconnPool->pop(dest, domain, retriable);
if (!Comm::IsConnOpen(conn)) {
// either there was no pconn to pop or this is not a retriable xaction
if (CachePeer *peer = dest->getPeer()) {
if (peer->standby.pool)
conn = peer->standby.pool->pop(dest, domain, true);
}
}
return conn; // open, closed, or nil
}
void
FwdState::initModule()
{
RegisterWithCacheManager();
}
void
FwdState::RegisterWithCacheManager(void)
{
Mgr::RegisterAction("forward", "Request Forwarding Statistics", fwdStats, 0, 1);
}
void
FwdState::logReplyStatus(int tries, const Http::StatusCode status)
{
if (status > Http::scInvalidHeader)
return;
assert(tries >= 0);
=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h 2016-01-15 06:47:59 +0000
+++ src/SquidConfig.h 2016-03-08 10:52:15 +0000
@@ -367,60 +367,61 @@
acl_access *brokenPosts;
#endif
acl_access *redirector;
acl_access *store_id;
acl_access *reply;
Acl::Address *outgoing_address;
#if USE_HTCP
acl_access *htcp;
acl_access *htcp_clr;
#endif
#if USE_OPENSSL
acl_access *ssl_bump;
#endif
#if FOLLOW_X_FORWARDED_FOR
acl_access *followXFF;
#endif /* FOLLOW_X_FORWARDED_FOR */
/// acceptible PROXY protocol clients
acl_access *proxyProtocol;
/// spoof_client_ip squid.conf acl.
/// nil unless configured
acl_access* spoof_client_ip;
acl_access *on_unsupported_protocol;
acl_access *ftp_epsv;
acl_access *forceRequestBodyContinuation;
+ acl_access *serverPconnForNonretriable;
} accessList;
AclDenyInfoList *denyInfoList;
struct {
size_t list_width;
int list_wrap;
char *anon_user;
int passive;
int epsv_all;
int epsv;
int eprt;
int sanitycheck;
int telnet;
} Ftp;
RefreshPattern *Refresh;
Store::DiskConfig cacheSwap;
struct {
char *directory;
int use_short_names;
} icons;
char *errorDirectory;
#if USE_ERR_LOCALES
char *errorDefaultLanguage;
int errorLogMissingLanguages;
#endif
char *errorStylesheet;
struct {
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre 2016-02-19 17:19:25 +0000
+++ src/cf.data.pre 2016-03-08 12:16:25 +0000
@@ -9605,31 +9605,69 @@
NAME: force_request_body_continuation
TYPE: acl_access
LOC: Config.accessList.forceRequestBodyContinuation
DEFAULT: none
DEFAULT_DOC: Deny, unless rules exist in squid.conf.
DOC_START
This option controls how Squid handles data upload requests from HTTP
and FTP agents that require a "Please Continue" control message response
to actually send the request body to Squid. It is mostly useful in
adaptation environments.
When Squid receives an HTTP request with an "Expect: 100-continue"
header or an FTP upload command (e.g., STOR), Squid normally sends the
request headers or FTP command information to an adaptation service (or
peer) and waits for a response. Most adaptation services (and some
broken peers) may not respond to Squid at that stage because they may
decide to wait for the HTTP request body or FTP data transfer. However,
that request body or data transfer may never come because Squid has not
responded with the HTTP 100 or FTP 150 (Please Continue) control message
to the request sender yet!
An allow match tells Squid to respond with the HTTP 100 or FTP 150
(Please Continue) control message on its own, before forwarding the
request to an adaptation service or peer. Such a response usually forces
the request sender to proceed with sending the body. A deny match tells
Squid to delay that control response until the origin server confirms
that the request body is needed. Delaying is the default behavior.
DOC_END
+NAME: server_pconn_for_nonretriable
+TYPE: acl_access
+DEFAULT: none
+DEFAULT_DOC: Open new connections for forwarding requests Squid cannot retry safely.
+LOC: Config.accessList.serverPconnForNonretriable
+DOC_START
+ This option provides fine-grained control over persistent connection
+ reuse when forwarding HTTP requests that Squid cannot retry. It is useful
+ in environments where opening new connections is very expensive
+ (e.g., all connections are secured with TLS with complex client and server
+ certificate validation) and race conditions associated with persistent
+ connections are very rare and/or only cause minor problems.
+
+ HTTP prohibits retrying unsafe and non-idempotent requests (e.g., POST).
+ Squid limitations also prohibit retrying all requests with bodies (e.g., PUT).
+ By default, when forwarding such "risky" requests, Squid opens a new
+ connection to the server or cache_peer, even if there is an idle persistent
+ connection available. When Squid is configured to risk sending a non-retriable
+ request on a previously used persistent connection, and the server closes
+ the connection before seeing that risky request, the user gets an error response
+ from Squid. In most cases, that error response will be HTTP 502 (Bad Gateway)
+ with ERR_ZERO_SIZE_OBJECT or ERR_WRITE_ERROR (peer connection reset) error detail.
+
+ If an allow rule matches, Squid reuses an available idle persistent connection
+ (if any) for the request that Squid cannot retry. If a deny rule matches, then
+ Squid opens a new connection for the request that Squid cannot retry.
+
+ This option does not affect requests that Squid can retry. They will reuse idle
+ persistent connections (if any).
+
+ This clause only supports fast acl types.
+ See http://wiki.squid-cache.org/SquidFaq/SquidAcl for details.
+
+ Example:
+ acl SpeedIsWorthTheRisk method POST
+ server_pconn_for_nonretriable allow SpeedIsWorthTheRisk
+DOC_END
+
EOF
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
