Purge map of failed auths periodically by keeping a last changed timestamp.

SyncRes thread local storage includes a map of failed auths which was
only cleaned if a specific IP was contacted again and that contact
succeeded. Persistent failing auths or auths that are never tried
again remained in the map.

While here add code to dump the failed servers map. Might (partially?)
solve #7771.

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index d36625e4d9d6686cbbb96e3cc0a20d4e82117a7d..0562ccb919dde1d244b04e5e4e6952168e4ad6a4 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -2844,7 +2844,8 @@ static void doStats(void)
 
     g_log<<Logger::Notice<<"stats: throttle map: "
       << broadcastAccFunction<uint64_t>(pleaseGetThrottleSize) <<", ns speeds: "
-      << broadcastAccFunction<uint64_t>(pleaseGetNsSpeedsSize)<<endl;
+      << broadcastAccFunction<uint64_t>(pleaseGetNsSpeedsSize)<<", failed ns: "
+      << broadcastAccFunction<uint64_t>(pleaseGetFailedServersSize)<<endl;
     g_log<<Logger::Notice<<"stats: outpacket/query ratio "<<(int)(SyncRes::s_outqueries*100.0/SyncRes::s_queries)<<"%";
     g_log<<Logger::Notice<<", "<<(int)(SyncRes::s_throttledqueries*100.0/(SyncRes::s_outqueries+SyncRes::s_throttledqueries))<<"% throttled, "
      <<SyncRes::s_nodelegated<<" no-delegation drops"<<endl;
@@ -2908,6 +2909,8 @@ static void houseKeeping(void *)
       if(!((cleanCounter++)%40)) {  // this is a full scan!
        time_t limit=now.tv_sec-300;
         SyncRes::pruneNSSpeeds(limit);
+        limit = now.tv_sec - SyncRes::s_serverdownthrottletime * 10;
+        SyncRes::pruneFailedServers(limit);
       }
       last_prune=time(0);
     }

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index a86500ba122f29e862b4681b04ed0fe25be23ed4..9ff72a13ade31da39df602b36a68b8eaab895bb2 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -231,6 +231,11 @@ static uint64_t* pleaseDumpThrottleMap(int fd)
   return new uint64_t(SyncRes::doDumpThrottleMap(fd));
 }
 
+static uint64_t* pleaseDumpFailedServers(int fd)
+{
+  return new uint64_t(SyncRes::doDumpFailedServers(fd));
+}
+
 template<typename T>
 static string doDumpNSSpeeds(T begin, T end)
 {
@@ -367,6 +372,28 @@ static string doDumpThrottleMap(T begin, T end)
   return "dumped "+std::to_string(total)+" records\n";
 }
 
+template<typename T>
+static string doDumpFailedServers(T begin, T end)
+{
+  T i=begin;
+  string fname;
+
+  if(i!=end)
+    fname=*i;
+
+  int fd=open(fname.c_str(), O_CREAT | O_EXCL | O_WRONLY, 0660);
+  if(fd < 0)
+    return "Error opening dump file for writing: "+stringerror()+"\n";
+  uint64_t total = 0;
+  try {
+    total = broadcastAccFunction<uint64_t>(boost::bind(pleaseDumpFailedServers, fd));
+  }
+  catch(...){}
+
+  close(fd);
+  return "dumped "+std::to_string(total)+" records\n";
+}
+
 uint64_t* pleaseWipeCache(const DNSName& canon, bool subtree)
 {
   return new uint64_t(t_RC->doWipeCache(canon, subtree));
@@ -885,6 +912,11 @@ static uint64_t getNsSpeedsSize()
   return broadcastAccFunction<uint64_t>(pleaseGetNsSpeedsSize);
 }
 
+uint64_t* pleaseGetFailedServersSize()
+{
+  return new uint64_t(SyncRes::getFailedServersSize());
+}
+
 uint64_t* pleaseGetConcurrentQueries()
 {
   return new uint64_t(getMT() ? getMT()->numProcesses() : 0);
@@ -1612,7 +1644,8 @@ string RecursorControlParser::getAnswer(const string& question, RecursorControlP
 "dump-edns [status] <filename>    dump EDNS status to the named file\n"
 "dump-nsspeeds <filename>         dump nsspeeds statistics to the named file\n"
 "dump-rpz <zone name> <filename>  dump the content of a RPZ zone to the named file\n"
-"dump-throttlemap <filename>      dump the contents of the throttle to the named file\n"
+"dump-throttlemap <filename>      dump the contents of the throttle map to the named file\n"
+"dump-failedservers <filename>    dump the failed servers to the named file\n"
 "get [key1] [key2] ..             get specific statistics\n"
 "get-all                          get all statistics\n"
 "get-dont-throttle-names          get the list of names that are not allowed to be throttled\n"
@@ -1684,6 +1717,9 @@ string RecursorControlParser::getAnswer(const string& question, RecursorControlP
   if(cmd=="dump-nsspeeds")
     return doDumpNSSpeeds(begin, end);
 
+  if(cmd=="dump-failedservers")
+    return doDumpFailedServers(begin, end);
+
   if(cmd=="dump-rpz") {
     return doDumpRPZ(begin, end);
   }

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 8bd1a6861da38c6b0fb9887057fe59f0c99fae14..2e3b03d1af106a1f4ebaca32b9c9e394546bf5aa 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -472,6 +472,27 @@ uint64_t SyncRes::doDumpThrottleMap(int fd)
   return count;
 }
 
+uint64_t SyncRes::doDumpFailedServers(int fd)
+{
+  auto fp = std::unique_ptr<FILE, int(*)(FILE*)>(fdopen(dup(fd), "w"), fclose);
+  if(!fp)
+    return 0;
+  fprintf(fp.get(), "; failed servers dump follows\n");
+  fprintf(fp.get(), "; remote IP\tcount\ttimestamp\n");
+  uint64_t count=0;
+
+  for(const auto& i : t_sstorage.fails.getMap())
+  {
+    count++;
+    char tmp[26];
+    ctime_r(&i.second.last, tmp);
+    fprintf(fp.get(), "%s\t%lld\t%s", i.first.toString().c_str(),
+            static_cast<long long>(i.second.value), tmp);
+  }
+
+  return count;
+}
+
 /* so here is the story. First we complete the full resolution process for a domain name. And only THEN do we decide
    to also do DNSSEC validation, which leads to new queries. To make this simple, we *always* ask for DNSSEC records
    so that if there are RRSIGs for a name, we'll have them.
@@ -3150,7 +3171,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
       t_sstorage.nsSpeeds[nsName.empty()? DNSName(remoteIP.toStringWithPort()) : nsName].submit(remoteIP, 1000000, &d_now); // 1 sec
 
       // code below makes sure we don't filter COM or the root
-      if (s_serverdownmaxfails > 0 && (auth != g_rootdnsname) && t_sstorage.fails.incr(remoteIP) >= s_serverdownmaxfails) {
+      if (s_serverdownmaxfails > 0 && (auth != g_rootdnsname) && t_sstorage.fails.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
         t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, "", 0), s_serverdownthrottletime, 10000);

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 3ce2bf04c0b28563f4db6731797b495b7bb09853..00220c9a70077cf773e93c03bf85102b03bed140 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -214,60 +214,74 @@ private:
 template<class Thing> class Counters : public boost::noncopyable
 {
 public:
-  Counters()
-  {
+  typedef unsigned long counter_t;
+  struct value_t {
+    counter_t value;
+    time_t last;
+  };
+  typedef std::map<Thing, value_t> cont_t;
+
+  cont_t getMap() const {
+    return d_cont;
   }
-  unsigned long value(const Thing& t) const
+  counter_t value(const Thing& t) const
   {
-    typename cont_t::const_iterator i=d_cont.find(t);
+    typename cont_t::const_iterator i = d_cont.find(t);
 
-    if(i==d_cont.end()) {
+    if (i == d_cont.end()) {
       return 0;
     }
-    return (unsigned long)i->second;
+    return i->second.value;
   }
-  unsigned long incr(const Thing& t)
+
+  counter_t incr(const Thing& t, const struct timeval & now)
   {
-    typename cont_t::iterator i=d_cont.find(t);
+    typename cont_t::iterator i = d_cont.find(t);
 
-    if(i==d_cont.end()) {
-      d_cont[t]=1;
+    if (i == d_cont.end()) {
+      d_cont[t].value = 1;
+      d_cont[t].last = now.tv_sec;
       return 1;
     }
     else {
-      if (i->second < std::numeric_limits<unsigned long>::max())
-        i->second++;
-      return (unsigned long)i->second;
-   }
+      if (i->second.value < std::numeric_limits<counter_t>::max()) {
+        i->second.value++;
+      }
+      i->second.last = now.tv_sec;
+      return i->second.value;
+    }
   }
-  unsigned long decr(const Thing& t)
-  {
-    typename cont_t::iterator i=d_cont.find(t);
 
-    if(i!=d_cont.end() && --i->second == 0) {
-      d_cont.erase(i);
-      return 0;
-    } else
-      return (unsigned long)i->second;
-  }
   void clear(const Thing& t)
   {
-    typename cont_t::iterator i=d_cont.find(t);
+    typename cont_t::iterator i = d_cont.find(t);
 
-    if(i!=d_cont.end()) {
+    if (i != d_cont.end()) {
       d_cont.erase(i);
     }
   }
+
   void clear()
   {
     d_cont.clear();
   }
+
   size_t size() const
   {
     return d_cont.size();
   }
+
+  void prune(time_t cutoff) {
+    for (auto it = d_cont.begin(); it != d_cont.end(); ) {
+      if (it->second.last <= cutoff) {
+        it = d_cont.erase(it);
+      } else {
+        ++it;
+      }
+    }
+  }
+
 private:
-  typedef map<Thing,unsigned long> cont_t;
   cont_t d_cont;
 };
 
@@ -406,6 +420,7 @@ public:
   static uint64_t doEDNSDump(int fd);
   static uint64_t doDumpNSSpeeds(int fd);
   static uint64_t doDumpThrottleMap(int fd);
+  static uint64_t doDumpFailedServers(int fd);
   static int getRootNS(struct timeval now, asyncresolve_t asyncCallback);
   static void clearDelegationOnly()
   {
@@ -526,6 +541,10 @@ public:
   {
     t_sstorage.fails.clear();
   }
+  static void pruneFailedServers(time_t cutoff)
+  {
+    t_sstorage.fails.prune(cutoff);
+  }
   static unsigned long getServerFailsCount(const ComboAddress& server)
   {
     return t_sstorage.fails.value(server);
@@ -1063,6 +1082,7 @@ template<class T> T broadcastAccFunction(const boost::function<T*()>& func);
 
 std::shared_ptr<SyncRes::domainmap_t> parseAuthAndForwards();
 uint64_t* pleaseGetNsSpeedsSize();
+uint64_t* pleaseGetFailedServersSize();
 uint64_t* pleaseGetCacheSize();
 uint64_t* pleaseGetNegCacheSize();
 uint64_t* pleaseGetCacheHits();