Also use a multi_index for teh failed counters class.

While there, stop using a template for the class itself. With only one
instantiation of the class, it does not provide any benefit.

pdns/syncres.cc

@@ -485,9 +485,9 @@ uint64_t SyncRes::doDumpFailedServers(int fd)
   {
     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);
+    ctime_r(&i.last, tmp);
+    fprintf(fp.get(), "%s\t%lld\t%s", i.address.toString().c_str(),
+            static_cast<long long>(i.value), tmp);
   }
 
   return count;

pdns/syncres.hh

@@ -211,54 +211,51 @@ private:
   bool d_needinit;
 };
 
-template<class Thing> class Counters : public boost::noncopyable
+class fails_t : public boost::noncopyable
 {
 public:
   typedef unsigned long counter_t;
   struct value_t {
-    counter_t value;
-    time_t last;
+    value_t(const ComboAddress &a) : address(a) {}
+    ComboAddress address;
+    mutable counter_t value{0};
+    time_t last{0};
   };
-  typedef std::map<Thing, value_t> cont_t;
+
+  typedef multi_index_container<value_t,
+                                indexed_by<
+                                  ordered_unique<tag<ComboAddress>, member<value_t, ComboAddress, &value_t::address>>,
+                                  ordered_non_unique<tag<time_t>, member<value_t, time_t, &value_t::last>>
+                                  >> cont_t;
 
   cont_t getMap() const {
     return d_cont;
   }
-  counter_t value(const Thing& t) const
+  counter_t value(const ComboAddress& t) const
   {
-    typename cont_t::const_iterator i = d_cont.find(t);
+    auto i = d_cont.find(t);
 
     if (i == d_cont.end()) {
       return 0;
     }
-    return i->second.value;
+    return i->value;
   }
 
-  counter_t incr(const Thing& t, const struct timeval & now)
+  counter_t incr(const ComboAddress& t, const struct timeval & now)
   {
-    typename cont_t::iterator i = d_cont.find(t);
+    auto i = d_cont.insert(t).first;
 
-    if (i == d_cont.end()) {
-      d_cont[t].value = 1;
-      d_cont[t].last = now.tv_sec;
-      return 1;
-    }
-    else {
-      if (i->second.value < std::numeric_limits<counter_t>::max()) {
-        i->second.value++;
-      }
-      i->second.last = now.tv_sec;
-      return i->second.value;
+    if (i->value < std::numeric_limits<counter_t>::max()) {
+      i->value++;
     }
+    auto &ind = d_cont.get<ComboAddress>();
+    ind.modify(i, [t = now.tv_sec](value_t &val) { val.last = t;});
+    return i->value;
   }
 
-  void clear(const Thing& t)
+  void clear(const ComboAddress& a)
   {
-    typename cont_t::iterator i = d_cont.find(t);
-
-    if (i != d_cont.end()) {
-      d_cont.erase(i);
-    }
+    d_cont.erase(a);
   }
 
   void clear()
@@ -272,13 +269,8 @@ public:
   }
 
   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;
-      }
-    }
+    auto &ind = d_cont.get<time_t>();
+    ind.erase(ind.begin(), ind.upper_bound(cutoff));
   }
 
 private:
@@ -418,7 +410,6 @@ public:
 
   typedef std::unordered_map<DNSName, AuthDomain> domainmap_t;
   typedef Throttle<boost::tuple<ComboAddress,DNSName,uint16_t> > throttle_t;
-  typedef Counters<ComboAddress> fails_t;
 
   struct ThreadLocalStorage {
     NegCache negcache;