Ignore comm_write notifications if the newly added

ICAPXaction::ignoreLastWrite member is set. This allows kids to more safely
work around comm inability to cancel a pending write request.

When connect() times out, throw an exception instead of calling mustStop()
because we can bypass the former and not the latter. We cannot bypass mustStop
because the code may use it for legitimate stopping conditions. Eventually,
we may want to have two kinds of exceptions: bypassable and fatal.

Support icap_connect_timeout and icap_io_timeout squid.conf options.

Count I/O timeout as a service failure. This helps suspend broken services
faster and avoid HTTP processing delays when the service is optional.

Eventually, we will probably count all (or most) exceptions as service
failures.

Removed ICAPXaction members that were copied to AsyncJob some time ago.

Polished debugging.

Merged from the squid3-icap branch.

diff --git a/src/ICAP/ICAPXaction.cc b/src/ICAP/ICAPXaction.cc
index 90b4f4403fedd7b81d13a934c53a4b091a2eee1c..be6f2e560b4b2c9209b848743632505598af3cdb 100644 (file)
--- a/src/ICAP/ICAPXaction.cc
+++ b/src/ICAP/ICAPXaction.cc
@@ -68,6 +68,7 @@ ICAPXaction::ICAPXaction(const char *aTypeName, ICAPInitiator *anInitiator, ICAP
         commEof(false),
         reuseConnection(true),
         isRetriable(true),
+        ignoreLastWrite(false),
         connector(NULL), reader(NULL), writer(NULL), closer(NULL)
 {
     debugs(93,3, typeName << " constructed, this=" << this <<
@@ -81,7 +82,7 @@ ICAPXaction::~ICAPXaction()
 }
 
 void ICAPXaction::disableRetries() {
-    debugs(93,5, typeName << (isRetriable ? "becomes" : "remains") <<
+    debugs(93,5, typeName << (isRetriable ? " becomes" : " remains") <<
         " final" << status());
     isRetriable = false;
 }
@@ -130,7 +131,8 @@ void ICAPXaction::openConnection()
 
     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;
@@ -225,12 +227,16 @@ void ICAPXaction::noteCommWrote(comm_err_t commStatus, size_t size)
 
     Must(writer);
     writer = NULL;
-
-    Must(commStatus == COMM_OK);
-
-    updateTimeout();
-
-    handleCommWrote(size);
+    
+    if (ignoreLastWrite) {
+        // a hack due to comm inability to cancel a pending write
+        ignoreLastWrite = false; 
+        debugs(93, 7, HERE << "ignoring last write; status: " << commStatus);
+    } else {
+        Must(commStatus == COMM_OK);
+        updateTimeout();
+        handleCommWrote(size);
+    }
 
     ICAPXaction_Exit();
 }
@@ -247,16 +253,14 @@ void ICAPXaction::noteCommTimedout()
 
 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());
-    }
+    service().noteFailure();
 
-    mustStop("connection with ICAP service timed out");
+    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
@@ -293,7 +297,8 @@ void ICAPXaction::updateTimeout() {
     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

diff --git a/src/ICAP/ICAPXaction.h b/src/ICAP/ICAPXaction.h
index 8630d92fe94bba1c3c869638b9ea34ee4cdeede7..d1485274f8c8f94f5eeb06d182d6bf446291af40 100644 (file)
--- a/src/ICAP/ICAPXaction.h
+++ b/src/ICAP/ICAPXaction.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: ICAPXaction.h,v 1.11 2007/05/08 16:32:12 rousskov Exp $
+ * $Id: ICAPXaction.h,v 1.12 2007/06/19 21:08:33 rousskov Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -132,6 +132,7 @@ protected:
     bool commEof;
     bool reuseConnection;
     bool isRetriable;
+    bool ignoreLastWrite;
 
     const char *stopReason;
 
@@ -141,13 +142,9 @@ protected:
     IOCB *writer;
     PF *closer;
 
-    const char *typeName; // the type of the final class (child), for debugging
-
 private:
     static int TheLastId;
 
-    const char *inCall; // name of the asynchronous call being executed, if any
-
     static void reusedConnection(void *data);
 
     //CBDATA_CLASS2(ICAPXaction);