Move fail_t maps to global shared instead of thread local

These maps are only used or modified when we're going out on the
net, so the performance impact of the locking should be relatively
low, while other threads could benefit greatly from information
learned by other threads.

Also, pruning of these data structure is cheap, so holding the lock
while pruning should be a short period of time.

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 0cac23188a5a7a39376b6044df36cdd0ccae5f3e..f7c3f4dfbd0b116bdc26e784fcc5d85d16053c29 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -386,7 +386,7 @@ static uint64_t* pleaseDumpNonResolvingNS(int fd)
 }
 
 // Generic dump to file command
-static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)(int s), const string& name)
+static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)(int s), const string& name, bool threads = true)
 {
   auto fdw = getfd(s);
 
@@ -396,8 +396,14 @@ static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)(
 
   uint64_t total = 0;
   try {
-    int fd = fdw;
-    total = broadcastAccFunction<uint64_t>([function, fd] { return function(fd); });
+    if (threads) {
+      int fd = fdw;
+      total = broadcastAccFunction<uint64_t>([function, fd] { return function(fd); });
+    } else {
+      auto ret = function(fdw);
+      total = *ret;
+      delete ret;
+    }
   }
   catch (std::exception& e) {
     return {1, name + ": error dumping data: " + string(e.what()) + "\n"};
@@ -1987,7 +1993,7 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str
     return doDumpToFile(s, pleaseDumpNSSpeeds, cmd);
   }
   if (cmd == "dump-failedservers") {
-    return doDumpToFile(s, pleaseDumpFailedServers, cmd);
+    return doDumpToFile(s, pleaseDumpFailedServers, cmd, false);
   }
   if (cmd == "dump-rpz") {
     return doDumpRPZ(s, begin, end);
@@ -1996,7 +2002,7 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str
     return doDumpToFile(s, pleaseDumpThrottleMap, cmd);
   }
   if (cmd == "dump-non-resolving") {
-    return doDumpToFile(s, pleaseDumpNonResolvingNS, cmd);
+    return doDumpToFile(s, pleaseDumpNonResolvingNS, cmd, false);
   }
   if (cmd == "wipe-cache" || cmd == "flushname") {
     return {0, doWipeCache(begin, end, 0xffff)};

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 4daa7440bd5e460caaddf6315e5ce6c3bf165a10..c36f6534636cb41ea72e3b9c18dd88f3b1beab84 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -48,6 +48,8 @@ EDNSSubnetOpts SyncRes::s_ecsScopeZero;
 string SyncRes::s_serverID;
 SyncRes::LogMode SyncRes::s_lm;
 const std::unordered_set<QType> SyncRes::s_redirectionQTypes = {QType::CNAME, QType::DNAME};
+LockGuarded<fails_t<ComboAddress>> SyncRes::s_fails;
+LockGuarded<fails_t<DNSName>> SyncRes::s_nonresolving;
 
 unsigned int SyncRes::s_maxnegttl;
 unsigned int SyncRes::s_maxbogusttl;
@@ -535,7 +537,7 @@ uint64_t SyncRes::doDumpFailedServers(int fd)
   fprintf(fp.get(), "; remote IP\tcount\ttimestamp\n");
   uint64_t count=0;
 
-  for(const auto& i : t_sstorage.fails.getMap())
+  for(const auto& i : s_fails.lock()->getMap())
   {
     count++;
     char tmp[26];
@@ -561,7 +563,7 @@ uint64_t SyncRes::doDumpNonResolvingNS(int fd)
   fprintf(fp.get(), "; name\tcount\ttimestamp\n");
   uint64_t count=0;
 
-  for(const auto& i : t_sstorage.nonresolving.getMap())
+  for(const auto& i : s_nonresolving.lock()->getMap())
   {
     count++;
     char tmp[26];
@@ -2357,7 +2359,7 @@ vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix,
   size_t nonresolvingfails = 0;
   if (!tns->first.empty()) {
     if (s_nonresolvingnsmaxfails > 0) {
-      nonresolvingfails = t_sstorage.nonresolving.value(tns->first);
+      nonresolvingfails = s_nonresolving.lock()->value(tns->first);
       if (nonresolvingfails >= s_nonresolvingnsmaxfails) {
         LOG(prefix<<qname<<": NS "<<tns->first<< " in non-resolving map, skipping"<<endl);
         return result;
@@ -2374,7 +2376,7 @@ vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix,
       if (s_nonresolvingnsmaxfails > 0 && d_outqueries > oldOutQueries) {
         auto dontThrottleNames = g_dontThrottleNames.getLocal();
         if (!dontThrottleNames->check(tns->first)) {
-          t_sstorage.nonresolving.incr(tns->first, d_now);
+          s_nonresolving.lock()->incr(tns->first, d_now);
         }
       }
       throw ex;
@@ -2383,12 +2385,12 @@ vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix,
       if (result.empty()) {
         auto dontThrottleNames = g_dontThrottleNames.getLocal();
         if (!dontThrottleNames->check(tns->first)) {
-          t_sstorage.nonresolving.incr(tns->first, d_now);
+          s_nonresolving.lock()->incr(tns->first, d_now);
         }
       }
       else if (nonresolvingfails > 0) {
         // Succeeding resolve, clear memory of recent failures
-        t_sstorage.nonresolving.clear(tns->first);
+        s_nonresolving.lock()->clear(tns->first);
       }
     }
     pierceDontQuery=false;
@@ -4056,7 +4058,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, d_now) >= s_serverdownmaxfails) {
+      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
         t_sstorage.throttle.throttle(d_now.tv_sec, boost::make_tuple(remoteIP, "", 0), s_serverdownthrottletime, 10000);
@@ -4112,7 +4114,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
 
   /* this server sent a valid answer, mark it backup up if it was down */
   if(s_serverdownmaxfails > 0) {
-    t_sstorage.fails.clear(remoteIP);
+    s_fails.lock()->clear(remoteIP);
   }
 
   if (lwr.d_tcbit) {

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 90b0259b43f8d3147ed738673b253942e1cec6b0..7f081d904b2525c868f0bc5718e41e1341935cd5 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -196,7 +196,7 @@ template<class T>
 class fails_t : public boost::noncopyable
 {
 public:
-  typedef unsigned long long counter_t;
+  typedef uint64_t counter_t;
   struct value_t {
     value_t(const T &a) : key(a) {}
     T key;
@@ -210,7 +210,7 @@ public:
                                   ordered_non_unique<tag<time_t>, member<value_t, time_t, &value_t::last>>
                                   >> cont_t;
 
-  cont_t getMap() const {
+  const cont_t& getMap() const {
     return d_cont;
   }
   counter_t value(const T& t) const
@@ -406,12 +406,13 @@ public:
 
   };
 
+  static LockGuarded<fails_t<ComboAddress>> s_fails;
+  static LockGuarded<fails_t<DNSName>> s_nonresolving;
+
   struct ThreadLocalStorage {
     nsspeeds_t nsSpeeds;
     throttle_t throttle;
     ednsstatus_t ednsstatus;
-    fails_t<ComboAddress> fails;
-    fails_t<DNSName> nonresolving;
     std::shared_ptr<domainmap_t> domainmap;
   };
 
@@ -542,31 +543,31 @@ public:
   }
   static uint64_t getFailedServersSize()
   {
-    return t_sstorage.fails.size();
+    return s_fails.lock()->size();
   }
   static uint64_t getNonResolvingNSSize()
   {
-    return t_sstorage.nonresolving.size();
+    return s_nonresolving.lock()->size();
   }
   static void clearFailedServers()
   {
-    t_sstorage.fails.clear();
+    s_fails.lock()->clear();
   }
   static void clearNonResolvingNS()
   {
-    t_sstorage.nonresolving.clear();
+    s_nonresolving.lock()->clear();
   }
   static void pruneFailedServers(time_t cutoff)
   {
-    t_sstorage.fails.prune(cutoff);
+    s_fails.lock()->prune(cutoff);
   }
   static unsigned long getServerFailsCount(const ComboAddress& server)
   {
-    return t_sstorage.fails.value(server);
+    return s_fails.lock()->value(server);
   }
   static void pruneNonResolving(time_t cutoff)
   {
-    t_sstorage.nonresolving.prune(cutoff);
+    s_nonresolving.lock()->prune(cutoff);
   }
   static void setDomainMap(std::shared_ptr<domainmap_t> newMap)
   {