Enforce connection re-try limit connect_retries

We used to have a maximum_single_address_tries limit which as similar
That is not dropped and the single-link retry used instead which applies
to ALL links regardless of the host IP address count since we dont know
that count any more.

diff --git a/doc/release-notes/release-3.2.sgml b/doc/release-notes/release-3.2.sgml
index 312258ce0cfc48e20210aafd13f0dca72125256f..55c72f50eb20c52283bac4d7c46acdd55bc06292 100644 (file)
--- a/doc/release-notes/release-3.2.sgml
+++ b/doc/release-notes/release-3.2.sgml
@@ -169,6 +169,10 @@ This section gives a thorough account of those changes in three categories:
        <p>Access control based on altered HTTP request following adaptation alterations (ICAP, eCAP, URL rewriter).
        An upgraded drop-in replacement for <em>http_access2</em> found in Squid-2.
 
+       <tag>connect_retries</tag>
+       <p>Replacement for <em>maximum_single_addr_tries</em>, but instead of only applying to hosts with single addresses.
+       This directive applies to all hosts, extending the number of connection attempts to each IP address.
+
        <tag>eui_lookup</tag>
        <p>Whether to lookup the EUI or MAC address of a connected client.
 
@@ -260,6 +264,10 @@ This section gives a thorough account of those changes in three categories:
        <tag>ftp_list_width</tag>
        <p>Obsolete.
 
+       <tag>maximum_single_addr_tries</tag>
+       <p>The behaviour controlled by this directive is no longer possible.
+       It has been replaced by <em>connect_retries</em> option which operates a little differently.
+
        <tag>url_rewrite_concurrency</tag>
        <p>Replaced by url_rewrite_children ... concurrency=N option.
 

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 16c6d65b95931416a621cb3cfd75f034018be24b..879a6f454cb402a77c0d735fb1e159a7f3bb19f3 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -526,12 +526,9 @@ configDoConfigure(void)
     else
         Config.appendDomainLen = 0;
 
-    if (Config.retry.maxtries > 10)
-        fatal("maximum_single_addr_tries cannot be larger than 10");
-
-    if (Config.retry.maxtries < 1) {
-        debugs(3, 0, "WARNING: resetting 'maximum_single_addr_tries to 1");
-        Config.retry.maxtries = 1;
+    if (Config.connect_retries > 10) {
+        debugs(0,DBG_CRITICAL, "WARNING: connect_retries cannot be larger than 10. Resetting to 10.");
+        Config.connect_retries = 10;
     }
 
     requirePathnameExists("MIME Config Table", Config.mimeTablePathname);

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 1e289011f880c9e9227ebd6880fc81f75d7e4e1a..c8af4a6856bf142a45e3222c751d53e501a1995f 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -2232,6 +2232,9 @@ LOC: Config.forward_max_tries
 DOC_START
        Controls how many different forward paths Squid will try
        before giving up. See also forward_timeout.
+       
+       NOTE: connect_retries (default: none) can make each of these
+       possible forwarding paths be tried multiple times.
 DOC_END
 
 NAME: hierarchy_stoplist
@@ -6785,21 +6788,26 @@ DOC_START
        see also refresh_pattern for a more selective approach.
 DOC_END
 
-NAME: maximum_single_addr_tries
+NAME: connect_retries
 TYPE: int
-LOC: Config.retry.maxtries
-DEFAULT: 1
+LOC: Config.connect_retries
+DEFAULT: 0
 DOC_START
-       This sets the maximum number of connection attempts for a
-       host that only has one address (for multiple-address hosts,
-       each address is tried once).
+       This sets the maximum number of connection attempts for each
+       potential host address selected by forwarding.
+
+       The default is not to re-try if the first connection attempt fails.
+       The (not recommended) maximum is 10 tries.
+
+       A warning message will be generated if it is set to a too-high
+       value and the configured value will be over-ridden.
 
-       The default value is one attempt, the (not recommended)
-       maximum is 255 tries.  A warning message will be generated
-       if it is set to a value greater than ten.
+       Note: These re-tries are in addition to forward_max_tries
+       which limit how many different addresses may be tried to find
+       a useful server.
 
-       Note: This is in addition to the request re-forwarding which
-       takes place if Squid fails to get a satisfying response.
+       The connect_retries * forward_max_tries attempts must all still
+       complete within the connection timeout period.
 DOC_END
 
 NAME: retry_on_error

diff --git a/src/comm/ConnectStateData.cc b/src/comm/ConnectStateData.cc
index 65271dc23cf9a0c29dfad7850fb0926fb5dd37ab..3f3005aa5600747059c102a4bead8d297f68629d 100644 (file)
--- a/src/comm/ConnectStateData.cc
+++ b/src/comm/ConnectStateData.cc
@@ -156,17 +156,20 @@ ConnectStateData::connect()
             netdbDeleteAddrNetwork(active->remote);
 #endif
 
-        // TODO: do the re-try logic with some sane bounds for handling many paths and retries.
-        if (fail_retries < Config.retry.maxtries)
-            eventAdd("ConnectStateData::Connect", ConnectStateData::Connect, this, 0.5, 0);
-        else if(squid_curtime - connstart > connect_timeout) {
+        // check for timeout FIRST.
+        if(squid_curtime - connstart > connect_timeout) {
             debugs(5, 5, HERE << "FD " << active->fd << ": * - ERR took too long already.");
             callCallback(COMM_TIMEOUT, errno);
+        } else if (fail_retries < Config.connect_retries) {
+            // check if connect_retries extends the single IP re-try limit.
+            eventAdd("ConnectStateData::Connect", ConnectStateData::Connect, this, 0.5, 0);
         } else if (paths && paths->size() > 0) {
+            // check if we have more maybe-useful paths to try.
             paths->shift();
             fail_retries = 0;
             eventAdd("ConnectStateData::Connect", ConnectStateData::Connect, this, 0.0, 0);
         } else {
+            // send ERROR back to the upper layer.
             debugs(5, 5, HERE << "FD " << active->fd << ": * - ERR tried too many times already.");
             callCallback(COMM_ERR_CONNECT, errno);
         }

diff --git a/src/structs.h b/src/structs.h
index 0ac8c99f3a7767e7f483dca2c08311de7654312f..95767be1b45dde43ba984083a935132e4820a270 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -443,6 +443,7 @@ struct SquidConfig {
     } onoff;
 
     int forward_max_tries;
+    int connect_retries;
 
     class ACL *aclList;
 
@@ -521,7 +522,6 @@ struct SquidConfig {
     char *errorStylesheet;
 
     struct {
-        int maxtries;
         int onerror;
     } retry;