Do not unconditionally revive dead peers after a DNS refresh.

Every hour, peerRefreshDNS() performs a DNS lookup of all cache_peer
addresses. Before this patch, even if the lookup results did not change,
the associated peerDNSConfigure() code silently cleared dead peer
marking (CachePeer::tcp_up counter), if any.  Forcefully reviving dead
peers every hour can lead to transaction delays (and delays may lead to
failures) due to connection timeouts when using a still dead peer.

This patch starts standard TCP probing (instead of pointless dead peer
reviving), correctly refreshing peer state.  The primary goal is to
cover a situation where a DNS refresh changes the peer address list.
However, TCP probing may be useful for other situations as well and has
low overhead (that is why it starts unconditionally). For example,
probing may be useful when the DNS refresh changes the order of IP
addresses. It also helps detecting dead idle peers.

Also delay and later resume peer probing if peerDNSConfigure() is
invoked when peers are being probed. Squid should re-probe because the
current probes may use stale IP addresses and produce wrong results.

diff --git a/src/CachePeer.cc b/src/CachePeer.cc
index 239d19b728b409d812d088fa8ffefca309fd0fdb..ff77552132bd45d238f11080ce164ecdb1f495c1 100644 (file)
--- a/src/CachePeer.cc
+++ b/src/CachePeer.cc
@@ -31,6 +31,7 @@ CachePeer::CachePeer() :
     digest_url(NULL),
 #endif
     tcp_up(0),
+    reprobe(false),
     n_addresses(0),
     rr_count(0),
     next(NULL),

diff --git a/src/CachePeer.h b/src/CachePeer.h
index ccfe212999120fcaddd69f369c249ecaa4f002cc..295a625606f63d13213dc711895fcdf2feb2266f 100644 (file)
--- a/src/CachePeer.h
+++ b/src/CachePeer.h
@@ -144,6 +144,8 @@ public:
 #endif
 
     int tcp_up;         /* 0 if a connect() fails */
+    /// whether to do another TCP probe after current TCP probes
+    bool reprobe;
 
     Ip::Address addresses[10];
     int n_addresses;

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 256ff9a56199a149e34ac7fe8033ff60d0a47b18..f1c91d9a8d870c5fb0df91871e36e771a20d884e 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -59,7 +59,7 @@ static void neighborAliveHtcp(CachePeer *, const MemObject *, const HtcpReplyDat
 static void neighborCountIgnored(CachePeer *);
 static void peerRefreshDNS(void *);
 static IPH peerDNSConfigure;
-static void peerProbeConnect(CachePeer *);
+static void peerProbeConnect(CachePeer *, const bool reprobeIfBusy = false);
 static CNCB peerProbeConnectDone;
 static void peerCountMcastPeersDone(void *data);
 static void peerCountMcastPeersStart(void *data);
@@ -1202,8 +1202,6 @@ peerDNSConfigure(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data
         return;
     }
 
-    p->tcp_up = p->connect_fail_limit;
-
     for (j = 0; j < (int) ia->count && j < PEER_MAX_ADDRESSES; ++j) {
         p->addresses[j] = ia->in_addrs[j];
         debugs(15, 2, "--> IP address #" << j << ": " << p->addresses[j]);
@@ -1214,6 +1212,8 @@ peerDNSConfigure(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data
     p->in_addr = p->addresses[0];
     p->in_addr.port(p->icp.port);
 
+    peerProbeConnect(p, true); // detect any died or revived peers ASAP
+
     if (p->type == PEER_MULTICAST)
         peerCountMcastPeersSchedule(p, 10);
 
@@ -1287,21 +1287,31 @@ peerConnectSucceded(CachePeer * p)
         p->tcp_up = p->connect_fail_limit;
 }
 
+/// whether new TCP probes are currently banned
+static bool
+peerProbeIsBusy(const CachePeer *p)
+{
+    if (p->testing_now > 0) {
+        debugs(15, 8, "yes, probing " << p);
+        return true;
+    }
+    if (squid_curtime - p->stats.last_connect_probe == 0) {
+        debugs(15, 8, "yes, just probed " << p);
+        return true;
+    }
+    return false;
+}
 /*
 * peerProbeConnect will be called on dead peers by neighborUp
 */
 static void
-peerProbeConnect(CachePeer * p)
+peerProbeConnect(CachePeer *p, const bool reprobeIfBusy)
 {
-    if (p->testing_now > 0) {
-        debugs(15, 8, "already probing " << p);
-        return;
-    }
-
-    if (squid_curtime - p->stats.last_connect_probe == 0) {
-        debugs(15, 8, "just probed " << p);
+    if (peerProbeIsBusy(p)) {
+        p->reprobe = reprobeIfBusy;
         return;
     }
+    p->reprobe = false;
 
     const time_t ctimeout = peerConnectTimeout(p);
     /* for each IP address of this CachePeer. find one that we can connect to and probe it. */
@@ -1337,6 +1347,9 @@ peerProbeConnectDone(const Comm::ConnectionPointer &conn, Comm::Flag status, int
     -- p->testing_now;
     conn->close();
     // TODO: log this traffic.
+
+    if (p->reprobe)
+        peerProbeConnect(p);
 }
 
 static void