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.h b/src/CachePeer.h
index 60520a982e3cb44e175754376cca43c0c9daa33d..7f28e7bf6c0a7b7453e1bf4a3b55494b53d7ce5b 100644 (file)
--- a/src/CachePeer.h
+++ b/src/CachePeer.h
@@ -145,6 +145,8 @@ public:
 #endif
 
     int tcp_up = 0;         /* 0 if a connect() fails */
+    /// whether to do another TCP probe after current TCP probes
+    bool reprobe = false;
 
     Ip::Address addresses[10];
     int n_addresses = 0;

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 891e23eb171459f38c369de4d0aae00619be0151..cb95fea621896e363a491aafd4d77154956db94d 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