--------------------- PatchSet 4752 Date: 2007/06/19 03:19:33 Author: rousskov Branch: squid3-icap Tag: (none) Log: Added icap_connect_timeout and icap_io_timeout squid.conf options to control how long an ICAP transaction should wait for the ICAP server to accept its connection or process the next I/O. These options are needed for many optional ICAP services that are poorly reachable or otherwise delay network I/Os. Waiting for a service is harmful to user experience, especially when the service failure or lack of connectivity can be bypassed. The two knobs use HTTP option values as defaults and have different defaults for essential and optional services. This may be a bad idea and will change depending on user feedback. All timeouts are currently global. Eventually, we will need per-service or service group timeouts and, possibly, even an OPTIONS-specific timeout. Also polished timeout debugging. Members: src/cf.data.pre:1.79.2.22->1.79.2.23 src/ICAP/ICAPConfig.cc:1.1.2.12->1.1.2.13 src/ICAP/ICAPConfig.h:1.1.2.7->1.1.2.8 src/ICAP/ICAPXaction.cc:1.1.2.23->1.1.2.24 Index: squid3/src/cf.data.pre =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/cf.data.pre,v retrieving revision 1.79.2.22 retrieving revision 1.79.2.23 diff -u -r1.79.2.22 -r1.79.2.23 --- squid3/src/cf.data.pre 1 Jun 2007 21:34:28 -0000 1.79.2.22 +++ squid3/src/cf.data.pre 19 Jun 2007 03:19:33 -0000 1.79.2.23 @@ -1,6 +1,6 @@ # -# $Id: cf.data.pre,v 1.79.2.22 2007/06/01 21:34:28 rousskov Exp $ +# $Id: cf.data.pre,v 1.79.2.23 2007/06/19 03:19:33 rousskov Exp $ # # # SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -4997,6 +4997,36 @@ If you want to enable the ICAP module support, set this to on. DOC_END +NAME: icap_connect_timeout +TYPE: time_t +DEFAULT: none +LOC: TheICAPConfig.connect_timeout_raw +IFDEF: ICAP_CLIENT +DOC_START + This parameter specifies how long to wait for the TCP connect to + the requested ICAP server to complete before giving up and either + terminating the HTTP transaction or bypassing the failure. + + The default for optional services is peer_connect_timeout. + The default for essential services is connect_timeout. + If this option is explicitly set, its value applies to all services. +DOC_END + +NAME: icap_io_timeout +COMMENT: time-units +TYPE: time_t +DEFAULT: none +LOC: TheICAPConfig.io_timeout_raw +IFDEF: ICAP_CLIENT +DOC_START + This parameter specifies how long to wait for an I/O activity on + an established, active ICAP connection before giving up and + either terminating the HTTP transaction or bypassing the + failure. + + The default is read_timeout. +DOC_END + NAME: icap_service_failure_limit TYPE: int IFDEF: ICAP_CLIENT Index: squid3/src/ICAP/ICAPConfig.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPConfig.cc,v retrieving revision 1.1.2.12 retrieving revision 1.1.2.13 diff -u -r1.1.2.12 -r1.1.2.13 --- squid3/src/ICAP/ICAPConfig.cc 1 Jun 2007 21:34:33 -0000 1.1.2.12 +++ squid3/src/ICAP/ICAPConfig.cc 19 Jun 2007 03:19:33 -0000 1.1.2.13 @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.cc,v 1.1.2.12 2007/06/01 21:34:33 rousskov Exp $ + * $Id: ICAPConfig.cc,v 1.1.2.13 2007/06/19 03:19:33 rousskov Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -431,3 +431,20 @@ classes.clean(); }; + +time_t ICAPConfig::connect_timeout(bool bypassable) const +{ + if (connect_timeout_raw > 0) + return connect_timeout_raw; // explicitly configured + + return bypassable ? Config.Timeout.peer_connect : Config.Timeout.connect; +} + +time_t ICAPConfig::io_timeout(bool) const +{ + if (io_timeout_raw > 0) + return io_timeout_raw; // explicitly configured + // TODO: provide a different default for an ICAP transaction that + // can still be bypassed + return Config.Timeout.read; +} Index: squid3/src/ICAP/ICAPConfig.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPConfig.h,v retrieving revision 1.1.2.7 retrieving revision 1.1.2.8 diff -u -r1.1.2.7 -r1.1.2.8 --- squid3/src/ICAP/ICAPConfig.h 1 Jun 2007 21:34:33 -0000 1.1.2.7 +++ squid3/src/ICAP/ICAPConfig.h 19 Jun 2007 03:19:33 -0000 1.1.2.8 @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.h,v 1.1.2.7 2007/06/01 21:34:33 rousskov Exp $ + * $Id: ICAPConfig.h,v 1.1.2.8 2007/06/19 03:19:33 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -96,6 +96,8 @@ int onoff; int preview_enable; int preview_size; + time_t connect_timeout_raw; + time_t io_timeout_raw; int default_options_ttl; int send_client_ip; int send_client_username; @@ -112,6 +114,9 @@ ~ICAPConfig(); + time_t connect_timeout(bool bypassable) const; + time_t io_timeout(bool bypassable) const; + void parseICAPService(void); void freeICAPService(void); void dumpICAPService(StoreEntry *, const char *); Index: squid3/src/ICAP/ICAPXaction.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPXaction.cc,v retrieving revision 1.1.2.23 retrieving revision 1.1.2.24 diff -u -r1.1.2.23 -r1.1.2.24 --- squid3/src/ICAP/ICAPXaction.cc 18 Jun 2007 20:58:46 -0000 1.1.2.23 +++ squid3/src/ICAP/ICAPXaction.cc 19 Jun 2007 03:19:33 -0000 1.1.2.24 @@ -131,7 +131,8 @@ debugs(93,3, typeName << " opens connection to " << s.host.buf() << ":" << s.port); - commSetTimeout(connection, Config.Timeout.connect, + // TODO: service bypass status may differ from that of a transaction + commSetTimeout(connection, TheICAPConfig.connect_timeout(service().bypass), &ICAPXaction_noteCommTimedout, this); closer = &ICAPXaction_noteCommClosed; @@ -252,16 +253,13 @@ void ICAPXaction::handleCommTimedout() { - debugs(93, 0, HERE << "ICAP FD " << connection << " timeout to " << theService->methodStr() << " " << theService->uri.buf()); + debugs(93, 2, HERE << typeName << " timeout with " << + theService->methodStr() << " " << theService->uri.buf() << status()); reuseConnection = false; - MemBuf mb; - mb.init(); - if (fillVirginHttpHeader(mb)) { - debugs(93, 0, HERE << "\tfor " << mb.content()); - } - - throw TexcHere("timed out while connecting to the ICAP service"); + throw TexcHere(connector ? + "timed out while connecting to the ICAP service" : + "timed out while talking to the ICAP service"); } // unexpected connection close while talking to the ICAP service @@ -298,7 +296,8 @@ if (reader || writer) { // restart the timeout before each I/O // XXX: why does Config.Timeout lacks a write timeout? - commSetTimeout(connection, Config.Timeout.read, + // TODO: service bypass status may differ from that of a transaction + commSetTimeout(connection, TheICAPConfig.io_timeout(service().bypass), &ICAPXaction_noteCommTimedout, this); } else { // clear timeout when there is no I/O