Enable length configuration for pipeline_prefetch queue

Updates the queue length magic numbers (2) by making pipeline_prefetch a
numeric queus length instead of aon/off toggle.

Also, adds configuration file consistency check for pipeline enabled
when client prsistent connections disable.

diff --git a/doc/release-notes/release-3.4.sgml b/doc/release-notes/release-3.4.sgml
index 677581501c432e5f160f8511d19dc273d2616e02..2c48e82490d031bf3848a304c2cf0f59799d742a 100644 (file)
--- a/doc/release-notes/release-3.4.sgml
+++ b/doc/release-notes/release-3.4.sgml
@@ -179,6 +179,9 @@ This section gives a thorough account of those changes in three categories:
        <p>New format code <em>%note</em> to log a transaction annotation linked to the
           transaction by ICAP, eCAP, a helper, or the <em>note</em> squid.conf directive.
 
+       <tag>pipeline_prefetch</tag>
+       <p>Updated to take a numeric count of prefetched pipeline requests instead of ON/OFF.
+
        <tag>unlinkd_program</tag>
        <p>New helper response format utilizing result codes <em>OK</em> and <em>BH</em>,
           to signal helper lookup results. Also, key-value response values to return

diff --git a/src/Parsing.cc b/src/Parsing.cc
index 1a764878d9eb69c1616c5532ecd885108259f3eb..b180eaa431e2ca2847380d22f902049de2038750 100644 (file)
--- a/src/Parsing.cc
+++ b/src/Parsing.cc
@@ -33,6 +33,7 @@
 #include "squid.h"
 #include "cache_cf.h"
 #include "compat/strtoll.h"
+#include "ConfigParser.h"
 #include "Parsing.h"
 #include "globals.h"
 #include "Debug.h"
@@ -161,7 +162,7 @@ GetInteger64(void)
 int
 GetInteger(void)
 {
-    char *token = strtok(NULL, w_space);
+    char *token = ConfigParser::strtokFile();
     int i;
 
     if (token == NULL)

diff --git a/src/SquidConfig.h b/src/SquidConfig.h
index 3d3cfaaf3bd1bf2c36a07e8b3c25c5556ff13126..a46a733cadc0ff92a75ab8c2688e47021444624d 100644 (file)
--- a/src/SquidConfig.h
+++ b/src/SquidConfig.h
@@ -332,7 +332,6 @@ public:
 
         int ie_refresh;
         int vary_ignore_expire;
-        int pipeline_prefetch;
         int surrogate_is_remote;
         int request_entities;
         int detect_broken_server_pconns;
@@ -361,6 +360,8 @@ public:
         int client_dst_passthru;
     } onoff;
 
+    int pipeline_max_prefetch;
+
     int forward_max_tries;
     int connect_retries;
 

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 265abd09c004085022c96da508da0469f32d45f6..d340169518eb257fe0d77fe4b6d2059c96fb07a5 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -965,6 +965,16 @@ configDoConfigure(void)
                (uint32_t)Config.maxRequestBufferSize, (uint32_t)Config.maxRequestHeaderSize);
     }
 
+    /*
+     * Disable client side request pipelining if client_persistent_connections OFF.
+     * Waste of resources queueing any pipelined requests when the first will close the connection.
+     */
+    if (Config.pipeline_max_prefetch > 0 && !Config.onoff.client_pconns) {
+        debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: pipeline_prefetch " << Config.pipeline_max_prefetch <<
+                   " requires client_persistent_connections ON. Forced pipeline_prefetch 0.");
+        Config.pipeline_max_prefetch = 0;
+    }
+
 #if USE_AUTH
     /*
      * disable client side request pipelining. There is a race with
@@ -973,12 +983,12 @@ configDoConfigure(void)
      * pipelining OFF, the client may fail to authenticate, but squid's
      * state will be preserved.
      */
-    if (Config.onoff.pipeline_prefetch) {
+    if (Config.pipeline_max_prefetch > 0) {
         Auth::Config *nego = Auth::Config::Find("Negotiate");
         Auth::Config *ntlm = Auth::Config::Find("NTLM");
         if ((nego && nego->active()) || (ntlm && ntlm->active())) {
-            debugs(3, DBG_IMPORTANT, "WARNING: pipeline_prefetch breaks NTLM and Negotiate authentication. Forced OFF.");
-            Config.onoff.pipeline_prefetch = 0;
+            debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: pipeline_prefetch breaks NTLM and Negotiate authentication. Forced pipeline_prefetch 0.");
+            Config.pipeline_max_prefetch = 0;
         }
     }
 #endif
@@ -2691,6 +2701,29 @@ parse_tristate(int *var)
 
 #define free_tristate free_int
 
+void
+parse_pipelinePrefetch(int *var)
+{
+    char *token = ConfigParser::strtokFile();
+
+    if (token == NULL)
+        self_destruct();
+
+    if (!strcmp(token, "on")) {
+        debugs(0, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: 'pipeline_prefetch on' is deprecated. Please update to use 1 (or a higher number).");
+        *var = 1;
+    } else if (!strcmp(token, "off")) {
+        debugs(0, DBG_PARSE_NOTE(2), "WARNING: 'pipeline_prefetch off' is deprecated. Please update to use '0'.");
+        *var = 0;
+    } else {
+        ConfigParser::strtokFileUndo();
+        parse_int(var);
+    }
+}
+
+#define free_pipelinePrefetch free_int
+#define dump_pipelinePrefetch dump_int
+
 static void
 dump_refreshpattern(StoreEntry * entry, const char *name, RefreshPattern * head)
 {

diff --git a/src/cf.data.depend b/src/cf.data.depend
index ece4bf2651746e3b64b9097798aeb8889aa623fa..c28b3e7586981dc149cc2eb624fd0126f05bf684 100644 (file)
--- a/src/cf.data.depend
+++ b/src/cf.data.depend
@@ -51,6 +51,7 @@ obsolete
 onoff
 peer
 peer_access            cache_peer acl
+pipelinePrefetch
 PortCfg
 QosConfig
 refreshpattern

diff --git a/src/cf.data.pre b/src/cf.data.pre
index c4d7626849f3b8650ae920d7d53c3891a7645ff7..c8cb4d13cf205f338d1617f649fdde50135f5065 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -8710,17 +8710,23 @@ DOC_START
 DOC_END
 
 NAME: pipeline_prefetch
-TYPE: onoff
-LOC: Config.onoff.pipeline_prefetch
-DEFAULT: off
+TYPE: pipelinePrefetch
+LOC: Config.pipeline_max_prefetch
+DEFAULT: 0
+DEFAULT_DOC: Do not pre-parse pipelined requests.
 DOC_START
-       To boost the performance of pipelined requests to closer
-       match that of a non-proxied environment Squid can try to fetch
-       up to two requests in parallel from a pipeline.
+       HTTP clients may send a pipeline of 1+N requests to Squid using a
+       single connection, without waiting for Squid to respond to the first
+       of those requests. This option limits the number of concurrent
+       requests Squid will try to handle in parallel. If set to N, Squid
+       will try to receive and process up to 1+N requests on the same
+       connection concurrently.
 
-       Defaults to off for bandwidth management and access logging
+       Defaults to 0 (off) for bandwidth management and access logging
        reasons.
 
+       NOTE: pipelining requires persistent connections to clients.
+
        WARNING: pipelining breaks NTLM and Negotiate/Kerberos authentication.
 DOC_END
 

diff --git a/src/client_side.cc b/src/client_side.cc
index 2b9341a2b5da3da60c49ee69d6a09f936664407d..805eb02fc77362bb307264c9b55135ea69a7a515 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2946,17 +2946,28 @@ connStripBufferWhitespace (ConnStateData * conn)
     }
 }
 
-static int
-connOkToAddRequest(ConnStateData * conn)
+/**
+ * Limit the number of concurrent requests.
+ * \return true  when there are available position(s) in the pipeline queue for another request.
+ * \return false when the pipeline queue is full or disabled.
+ */
+bool
+ConnStateData::concurrentRequestQueueFilled() const
 {
-    int result = conn->getConcurrentRequestCount() < (Config.onoff.pipeline_prefetch ? 2 : 1);
+    const int existingRequestCount = getConcurrentRequestCount();
+
+    // default to the configured pipeline size.
+    // add 1 because the head of pipeline is counted in concurrent requests and not prefetch queue
+    const int concurrentRequestLimit = Config.pipeline_max_prefetch + 1;
 
-    if (!result) {
-        debugs(33, 3, HERE << conn->clientConnection << " max concurrent requests reached");
-        debugs(33, 5, HERE << conn->clientConnection << " defering new request until one is done");
+    // when queue filled already we cant add more.
+    if (existingRequestCount >= concurrentRequestLimit) {
+        debugs(33, 3, clientConnection << " max concurrent requests reached (" << concurrentRequestLimit << ")");
+        debugs(33, 5, clientConnection << " deferring new request until one is done");
+        return true;
     }
 
-    return result;
+    return false;
 }
 
 /**
@@ -2981,10 +2992,9 @@ ConnStateData::clientParseRequests()
         if (in.notYetUsed == 0)
             break;
 
-        /* Limit the number of concurrent requests to 2 */
-        if (!connOkToAddRequest(this)) {
+        /* Limit the number of concurrent requests */
+        if (concurrentRequestQueueFilled())
             break;
-        }
 
         /* Should not be needed anymore */
         /* Terminate the string */

diff --git a/src/client_side.h b/src/client_side.h
index ffe33e58ce46cb157a92b88e51ab7c71af393be4..310de54b8ea3e18fb011860068a082e802c58908 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -178,7 +178,7 @@ class ServerBump;
 /**
  * Manages a connection to a client.
  *
- * Multiple requests (up to 2) can be pipelined. This object is responsible for managing
+ * Multiple requests (up to pipeline_prefetch) can be pipelined. This object is responsible for managing
  * which one is currently being fulfilled and what happens to the queue if the current one
  * causes the client connection to be closed early.
  *
@@ -392,6 +392,7 @@ private:
     int connReadWasError(comm_err_t flag, int size, int xerrno);
     int connFinishedWithConn(int size);
     void clientAfterReadingRequests();
+    bool concurrentRequestQueueFilled() const;
 
 #if USE_AUTH
     /// some user details that can be used to perform authentication on this connection