Remember reason we're throttling

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index b504f66fd41969b8d5da27e3c18ac5a63362530e..1e5864aaa9e80b539a604194bbcf3711201ed0b8 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -256,15 +256,18 @@ public:
   Throttle(const Throttle&) = delete;
 
   using Key = std::tuple<ComboAddress, DNSName, QType>;
+  using Reason = SyncRes::ThrottleReason;
+
   struct entry_t
   {
-    entry_t(Key  thing_, time_t ttd_, unsigned int count_) :
-      thing(std::move(thing_)), ttd(ttd_), count(count_)
+    entry_t(Key thing_, time_t ttd_, unsigned int count_, Reason reason_) :
+      thing(std::move(thing_)), ttd(ttd_), count(count_), reason(reason_)
     {
     }
     Key thing;
     time_t ttd;
     mutable unsigned int count;
+    Reason reason;
   };
   using cont_t = multi_index_container<entry_t,
                                        indexed_by<
@@ -286,18 +289,22 @@ public:
     return true; // still listed, still blocked
   }
 
-  void throttle(time_t now, const Key& arg, time_t ttl, unsigned int count)
+  void throttle(time_t now, const Key& arg, time_t ttl, unsigned int count, Reason reason)
   {
     auto iter = d_cont.find(arg);
     time_t ttd = now + ttl;
     if (iter == d_cont.end()) {
-      d_cont.emplace(arg, ttd, count);
+      d_cont.emplace(arg, ttd, count, reason);
     }
     else if (ttd > iter->ttd || count > iter->count) {
       ttd = std::max(iter->ttd, ttd);
       count = std::max(iter->count, count);
       auto& ind = d_cont.template get<Key>();
-      ind.modify(iter, [ttd, count](entry_t& entry) { entry.ttd = ttd; entry.count = count; });
+      ind.modify(iter, [ttd, count, reason](entry_t& entry) {
+        entry.ttd = ttd;
+        entry.count = count;
+        entry.reason = reason;
+      });
     }
   }
 
@@ -326,6 +333,26 @@ public:
     ind.erase(ind.begin(), ind.upper_bound(now));
   }
 
+  static std::string toString(Reason reason)
+  {
+    static const std::array<std::string, 10> reasons = {
+      "None",
+      "ServerDown",
+      "PermanentError",
+      "Timeout",
+      "ParseError",
+      "RCodeServFail",
+      "RCodeRefused",
+      "RCodeOther",
+      "TCPTruncate",
+      "Lame"};
+    const auto index = static_cast<unsigned int>(reason);
+    if (index >= reasons.size()) {
+      return "?";
+    }
+    return reasons.at(index);
+  }
+
 private:
   cont_t d_cont;
 };
@@ -1286,14 +1313,14 @@ void SyncRes::unThrottle(const ComboAddress& server, const DNSName& name, QType
   s_throttle.lock()->clear(std::tuple(server, name, qtype));
 }
 
-void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries)
+void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, Throttle::Reason reason)
 {
-  s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries);
+  s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries, reason);
 }
 
-void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries)
+void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, Throttle::Reason reason)
 {
-  s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries);
+  s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries, reason);
 }
 
 uint64_t SyncRes::doDumpThrottleMap(int fileDesc)
@@ -1308,7 +1335,7 @@ uint64_t SyncRes::doDumpThrottleMap(int fileDesc)
     return 0;
   }
   fprintf(filePtr.get(), "; throttle map dump follows\n");
-  fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\n");
+  fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\treason\n");
   uint64_t count = 0;
 
   // Get a copy to avoid holding the lock while doing I/O
@@ -1316,8 +1343,8 @@ uint64_t SyncRes::doDumpThrottleMap(int fileDesc)
   for (const auto& iter : throttleMap) {
     count++;
     timebuf_t tmp;
-    // remote IP, dns name, qtype, count, ttd
-    fprintf(filePtr.get(), "%s\t%s\t%s\t%u\t%s\n", std::get<0>(iter.thing).toString().c_str(), std::get<1>(iter.thing).toLogString().c_str(), std::get<2>(iter.thing).toString().c_str(), iter.count, timestamp(iter.ttd, tmp));
+    // remote IP, dns name, qtype, count, ttd, reason
+    fprintf(filePtr.get(), "%s\t%s\t%s\t%u\t%s\t%s\n", std::get<0>(iter.thing).toString().c_str(), std::get<1>(iter.thing).toLogString().c_str(), std::get<2>(iter.thing).toString().c_str(), iter.count, timestamp(iter.ttd, tmp), Throttle::toString(iter.reason).c_str());
   }
 
   return count;
@@ -5422,11 +5449,11 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
       if (s_serverdownmaxfails > 0 && auth != g_rootdnsname && s_fails.lock()->incr(remoteIP, d_now) >= s_serverdownmaxfails) {
         LOG(prefix << qname << ": Max fails reached resolving on " << remoteIP.toString() << ". Going full throttle for " << s_serverdownthrottletime << " seconds" << endl);
         // mark server as down
-        doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000);
+        doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000, Throttle::Reason::ServerDown);
       }
       else if (resolveret == LWResult::Result::PermanentError) {
         // unreachable, 1 minute or 100 queries
-        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100);
+        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100, Throttle::Reason::PermanentError);
       }
       else {
         // If the actual response time was more than 80% of the default timeout, we throttle. On a
@@ -5434,7 +5461,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
         // such a shortened timeout.
         if (responseUsec > g_networkTimeoutMsec * 800) {
           // timeout, 10 seconds or 5 queries
-          doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5);
+          doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5, Throttle::Reason::Timeout);
         }
       }
     }
@@ -5451,10 +5478,10 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
 
       if (doTCP) {
         // we can be more heavy-handed over TCP
-        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10);
+        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10, Throttle::Reason::ParseError);
       }
       else {
-        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2);
+        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2, Throttle::Reason::ParseError);
       }
     }
     return false;
@@ -5470,7 +5497,19 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
         s_nsSpeeds.lock()->find_or_enter(nsName.empty() ? DNSName(remoteIP.toStringWithPort()) : nsName, d_now).submit(remoteIP, 1000000, d_now); // 1 sec
       }
       else {
-        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3);
+        Throttle::Reason reason{};
+        switch (lwr.d_rcode) {
+        case RCode::ServFail:
+          reason = Throttle::Reason::RCodeServFail;
+          break;
+        case RCode::Refused:
+          reason = Throttle::Reason::RCodeRefused;
+          break;
+        default:
+          reason = Throttle::Reason::RCodeOther;
+          break;
+        }
+        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, reason);
       }
     }
     return false;
@@ -5490,7 +5529,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
       LOG(prefix << qname << ": Truncated bit set, over TCP?" << endl);
       if (!dontThrottle) {
         /* let's treat that as a ServFail answer from this server */
-        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3);
+        doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, Throttle::Reason::TCPTruncate);
       }
       return false;
     }
@@ -5896,7 +5935,7 @@ int SyncRes::doResolveAt(NsSet& nameservers, DNSName auth, bool flawedNSSet, con
           }
           /* was lame */
           if (!shouldNotThrottle(&tns->first, &*remoteIP)) {
-            doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100);
+            doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100, Throttle::Reason::Lame);
           }
         }
 

diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
index ebf0005e98933a8d238ef7fc7c37ba7a7b0b86aa..09db4ac97668daf309fea7b27675d4794445d011 100644 (file)
--- a/pdns/recursordist/syncres.hh
+++ b/pdns/recursordist/syncres.hh
@@ -255,8 +255,22 @@ public:
   static void clearThrottle();
   static bool isThrottled(time_t now, const ComboAddress& server, const DNSName& target, QType qtype);
   static bool isThrottled(time_t now, const ComboAddress& server);
-  static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries);
-  static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries);
+
+  enum class ThrottleReason : uint8_t
+  {
+    None,
+    ServerDown,
+    PermanentError,
+    Timeout,
+    ParseError,
+    RCodeServFail,
+    RCodeRefused,
+    RCodeOther,
+    TCPTruncate,
+    Lame,
+  };
+  static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, ThrottleReason reason);
+  static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, ThrottleReason reason);
   static void unThrottle(const ComboAddress& server, const DNSName& qname, QType qtype);
 
   static uint64_t getFailedServersSize();
@@ -894,7 +908,7 @@ class ImmediateServFailException
 {
 public:
   ImmediateServFailException(string reason_) :
-    reason(std::move(reason_)){};
+    reason(std::move(reason_)) {};
 
   string reason; //! Print this to tell the user what went wrong
 };

diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc
index 7f57813e6b07d248426af1e5cad246fa056680cc..dc0fc76d58ae45e48f393f71d368ca427b5b2e77 100644 (file)
--- a/pdns/recursordist/test-syncres_cc2.cc
+++ b/pdns/recursordist/test-syncres_cc2.cc
@@ -410,7 +410,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server)
 
   /* mark ns as down */
   time_t now = sr->getNow().tv_sec;
-  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000);
+  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000, SyncRes::ThrottleReason::Timeout);
 
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
@@ -432,7 +432,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_count)
   const size_t blocks = 10;
   /* mark ns as down for 'blocks' queries */
   time_t now = sr->getNow().tv_sec;
-  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks);
+  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks, SyncRes::ThrottleReason::Timeout);
 
   for (size_t idx = 0; idx < blocks; idx++) {
     BOOST_CHECK(SyncRes::isThrottled(now, ns));
@@ -454,7 +454,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_time)
   const size_t seconds = 1;
   /* mark ns as down for 'seconds' seconds */
   time_t now = sr->getNow().tv_sec;
-  SyncRes::doThrottle(now, ns, seconds, 10000);
+  SyncRes::doThrottle(now, ns, seconds, 10000, SyncRes::ThrottleReason::Timeout);
 
   BOOST_CHECK(SyncRes::isThrottled(now, ns));