From: rousskov <>
Date: Wed, 20 Jun 2007 03:08:33 +0000 (+0000)
Subject: Ignore comm_write notifications if the newly added
X-Git-Tag: SQUID_3_0_PRE7~211
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cfc684056b06a337844889663f0d0fadc1af9c33;p=thirdparty%2Fsquid.git

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 90b4f4403f..be6f2e560b 100644
--- 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 8630d92fe9..d1485274f8 100644
--- 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);