Refactor stats queue, introducing a pointor to a function that does the work

(pointing to a resolve function) and almost-expired specific stats.

diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc
index 0485419824bc0c177d82030e9de03e32eb4201ac..d22447f8364f6ed445d36cd61efa8829047db49c 100644 (file)
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -70,7 +70,7 @@ bool RecursorPacketCache::checkResponseMatches(std::pair<packetCache_t::index<Ha
         const bool almostExpired = ttl <= deadline;
         if (almostExpired) {
           iter->d_submitted = true;
-          pushTask(qname, qtype, iter->d_ttd);
+          pushAlmostExpiredTask(qname, qtype, iter->d_ttd);
         }
       }
       *responsePacket = iter->d_packet;

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 76e702fba1065960af59f6c17eb840ea3d1d24fe..b505b2eb106317d1f79b560421ed618c9902744c 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -248,7 +248,7 @@ time_t MemRecursorCache::fakeTTD(MemRecursorCache::OrderedTagIterator_t& entry,
         return -1;
       } else {
         if (!entry->d_submitted) {
-          pushTask(qname, qtype, entry->d_ttd);
+          pushAlmostExpiredTask(qname, qtype, entry->d_ttd);
           entry->d_submitted = true;
         }
       }

diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am
index 8309c9f72954772a004574c606eedd0cc53f9c1c..80939a72f94f03d8f37c48af1e64fee816ec46b0 100644 (file)
--- a/pdns/recursordist/Makefile.am
+++ b/pdns/recursordist/Makefile.am
@@ -183,6 +183,7 @@ pdns_recursor_SOURCES = \
        sortlist.cc sortlist.hh \
        sstuff.hh \
        stable-bloom.hh \
+       stats_t.hh \
        svc-records.cc svc-records.hh \
        syncres.cc syncres.hh \
        taskqueue.cc taskqueue.hh \

diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc
index 6b33aba0c2624092a20a0ee9f18b2fc92f1548e8..21404ad13b987497f42a1c424f6e3bf76abbd01a 100644 (file)
--- a/pdns/recursordist/rec-taskqueue.cc
+++ b/pdns/recursordist/rec-taskqueue.cc
@@ -21,18 +21,62 @@
  */
 #include "rec-taskqueue.hh"
 #include "taskqueue.hh"
+#include "logger.hh"
+#include "stat_t.hh"
 #include "syncres.hh"
 
 static thread_local pdns::TaskQueue t_taskQueue;
+static pdns::stat_t s_almost_expired_tasks_pushed;
+static pdns::stat_t s_almost_expired_tasks_run;
+static pdns::stat_t s_almost_expired_tasks_exceptions;
+
+static void resolve(const struct timeval &now, bool logErrors, const pdns::ResolveTask& task)
+{
+  ++s_almost_expired_tasks_run;
+  const string msg = "Exception while running a background ResolveTask";
+  SyncRes sr(now);
+  vector<DNSRecord> ret;
+  sr.setRefreshAlmostExpired(task.d_refreshMode);
+  try {
+    g_log << Logger::Debug << "TaskQueue: resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << endl;
+    int res = sr.beginResolve(task.d_qname, QType(task.d_qtype), QClass::IN, ret);
+    g_log << Logger::Debug << "TaskQueue: DONE resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << ": " << res << endl;
+  }
+  catch (const std::exception& e) {
+    ++s_almost_expired_tasks_exceptions;
+    g_log << Logger::Error << msg << ": " << e.what() << endl;
+  }
+  catch (const PDNSException& e) {
+    ++s_almost_expired_tasks_exceptions;
+    g_log << Logger::Notice << msg << ": " << e.reason << endl;
+  }
+  catch (const ImmediateServFailException& e) {
+    ++s_almost_expired_tasks_exceptions;
+    if (logErrors) {
+      g_log << Logger::Notice << msg << ": " << e.reason << endl;
+    }
+  }
+  catch (const PolicyHitException& e) {
+    ++s_almost_expired_tasks_exceptions;
+    if (logErrors) {
+      g_log << Logger::Notice << msg << ": PolicyHit" << endl;
+    }
+  }
+  catch (...) {
+    ++s_almost_expired_tasks_exceptions;
+    g_log << Logger::Error << msg << endl;
+  }
+}
 
 void runTaskOnce(bool logErrors)
 {
   t_taskQueue.runOnce(logErrors);
 }
 
-void pushTask(const DNSName& qname, uint16_t qtype, time_t deadline)
+void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline)
 {
-  t_taskQueue.push({qname, qtype, deadline, true});
+  ++s_almost_expired_tasks_pushed;
+  t_taskQueue.push({qname, qtype, deadline, true, resolve});
 }
 
 uint64_t getTaskPushes()
@@ -49,3 +93,19 @@ uint64_t getTaskSize()
 {
   return broadcastAccFunction<uint64_t>([] { return t_taskQueue.getSize(); });
 }
+
+uint64_t getAlmostExpiredTasksPushed()
+{
+  return s_almost_expired_tasks_pushed;
+}
+
+uint64_t getAlmostExpiredTasksRun()
+{
+  return s_almost_expired_tasks_run;
+}
+
+uint64_t getAlmostExpiredTaskExceptions()
+{
+  return s_almost_expired_tasks_exceptions;
+}
+

diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh
index 0bcaf70ab3e113556e3cf9c31f78e07fe4547578..2e9df555233e941feb43c7a3db8bd9ce79b2580b 100644 (file)
--- a/pdns/recursordist/rec-taskqueue.hh
+++ b/pdns/recursordist/rec-taskqueue.hh
@@ -24,7 +24,13 @@
 #include "dnsname.hh"
 
 void runTaskOnce(bool logErrors);
-void pushTask(const DNSName& qname, uint16_t qtype, time_t deadline);
+void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline);
+// General task stats
 uint64_t getTaskPushes();
 uint64_t getTaskExpired();
 uint64_t getTaskSize();
+
+// Almost expired specific stats
+uint64_t getAlmostExpiredTasksPushed();
+uint64_t getAlmostExpiredTasksRun();
+uint64_t getAlmostExpiredTaskExceptions();

diff --git a/pdns/recursordist/stat_t.hh b/pdns/recursordist/stat_t.hh
new file mode 120000 (symlink)
index 0000000..0b8289c
--- /dev/null
+++ b/pdns/recursordist/stat_t.hh
@@ -0,0 +1 @@
+../stat_t.hh
\ No newline at end of file

diff --git a/pdns/recursordist/taskqueue.cc b/pdns/recursordist/taskqueue.cc
index 0a5c405189d62f207fe6c9b027c5d5959786c7ec..c1aa80c38f1e8fc60419ee353d6943edda32cc94 100644 (file)
--- a/pdns/recursordist/taskqueue.cc
+++ b/pdns/recursordist/taskqueue.cc
@@ -38,7 +38,7 @@ size_t TaskQueue::size() const
   return d_queue.size();
 }
 
-void TaskQueue::push(const ResolveTask&& task)
+void TaskQueue::push(ResolveTask&& task)
 {
   // Insertion fails if it's already there, no problem since we're already scheduled
   // and the deadline would remain the same anyway.
@@ -64,33 +64,7 @@ bool TaskQueue::runOnce(bool logErrors)
   struct timeval now;
   gettimeofday(&now, 0);
   if (task.d_deadline >= now.tv_sec) {
-    SyncRes sr(now);
-    vector<DNSRecord> ret;
-    sr.setRefreshAlmostExpired(task.d_refreshMode);
-    try {
-      g_log << Logger::Debug << "TaskQueue: resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << endl;
-      int res = sr.beginResolve(task.d_qname, QType(task.d_qtype), QClass::IN, ret);
-      g_log << Logger::Debug << "TaskQueue: DONE resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << ": " << res << endl;
-    }
-    catch (const std::exception& e) {
-      g_log << Logger::Error << "Exception while running the background task queue: " << e.what() << endl;
-    }
-    catch (const PDNSException& e) {
-      g_log << Logger::Notice << "Exception while running the background task queue: " << e.reason << endl;
-    }
-    catch (const ImmediateServFailException& e) {
-      if (logErrors) {
-        g_log << Logger::Notice << "Exception while running the background task queue: " << e.reason << endl;
-      }
-    }
-    catch (const PolicyHitException& e) {
-      if (logErrors) {
-        g_log << Logger::Notice << "Policy hit while running the background task queue" << endl;
-      }
-    }
-    catch (...) {
-      g_log << Logger::Error << "Exception while running the background task queue" << endl;
-    }
+    task.func(now, logErrors, task);
   }
   else {
     // Deadline passed

diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh
index 5ebb3aece36a9b561e37de87f07112c43da8bca4..ec0c91d08894a7897202e5a01262bdbe0c519cb6 100644 (file)
--- a/pdns/recursordist/taskqueue.hh
+++ b/pdns/recursordist/taskqueue.hh
@@ -39,12 +39,14 @@ using namespace ::boost::multi_index;
 namespace pdns
 {
 
+// ATM we have one task type, if we get more, the unique key in the index needs to be adapted
 struct ResolveTask
 {
   DNSName d_qname;
   uint16_t d_qtype;
   time_t d_deadline;
   bool d_refreshMode; // Whether to run this task in regular mode (false) or in the mode that refreshes almost expired tasks
+  void (*func)(const struct timeval&, bool logErrors, const ResolveTask&);
 };
 
 struct HashTag
@@ -70,7 +72,7 @@ class TaskQueue
 public:
   bool empty() const;
   size_t size() const;
-  void push(const ResolveTask&& task);
+  void push(ResolveTask&& task);
   ResolveTask pop();
   bool runOnce(bool logErrors); // Run one task if the queue is not empty
   void runAll(bool logErrors);

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 6a94f060e30ba8e7845c4d03bcd55f412410f7cb..6a461506393e1d0b4a8b33a218e352b21f99e528 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -544,7 +544,7 @@ LWResult::Result basicRecordsForQnameMinimization(LWResult* res, const DNSName&
 
 pdns::TaskQueue g_test_tasks;
 
-void pushTask(const DNSName& qname, uint16_t qtype, time_t deadline)
+void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline)
 {
-  g_test_tasks.push({qname, qtype, deadline, true});
+  g_test_tasks.push({qname, qtype, deadline, true, nullptr});
 }