From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 12 Apr 2022 10:25:17 +0000 (+0200)
Subject: Review comments: document what happens on failure and use runOnce() as a building... 
X-Git-Tag: rec-4.7.0-beta1^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9da2cc318fb2a9662772499e15db59a0e1748d3c;p=thirdparty%2Fpdns.git

Review comments: document what happens on failure and use runOnce() as a building block for runTasks()
---

diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst
index 7773ac9b84..802b17327f 100644
--- a/pdns/recursordist/docs/settings.rst
+++ b/pdns/recursordist/docs/settings.rst
@@ -1079,12 +1079,14 @@ Limit the maximum number of simultaneous DoT probes the Recursor will schedule.
 The default value 0 means no DoT probes are scheduled.
 
 DoT probes are used to check if an authoritative server's IP address supports DoT.
-If the probe determines an IP address supports DoT, the Recursor will use DoT to contact it for subsequent queries.
+If the probe determines an IP address supports DoT, the Recursor will use DoT to contact it for subsequent queries until a failure occurs.
+After a failure, the Recursor will stop using DoT for that specific IP address for a while.
 The results of probes are remembered and can be viewed by the ``rec_control dump-dot-probe-map`` command.
 If the maximum number of pending probes is reached, no probes will be scheduled, even if no DoT status is known for an address.
 If the result of a probe is not yet available, the Recursor will contact the authoritative server in the regular way, unless an authoritative server is configured to be contacted over DoT always using :ref:`setting-dot-to-auth-names`.
 In that case no probe will be scheduled.
 
+
 Note::
   DoT probing is an experimental feature.
   Please test thoroughly if it is suitable in your specific production environment before enabling.
diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc
index a4ba8f88ae..98b5f6296d 100644
--- a/pdns/recursordist/rec-taskqueue.cc
+++ b/pdns/recursordist/rec-taskqueue.cc
@@ -207,28 +207,20 @@ static void tryDoT(const struct timeval& now, bool logErrors, const pdns::Resolv
 void runTasks(size_t max, bool logErrors)
 {
   for (size_t count = 0; count < max; count++) {
-    pdns::ResolveTask task;
-    {
-      auto lock = s_taskQueue.lock();
-      if (lock->queue.empty()) {
-        return;
-      }
-      task = lock->queue.pop();
-    }
-    bool expired = task.run(logErrors);
-    if (expired) {
-      s_taskQueue.lock()->queue.incExpired();
+    if (!runTaskOnce(logErrors)) {
+      // No more tasks in queue
+      break;
     }
   }
 }
 
-void runTaskOnce(bool logErrors)
+bool runTaskOnce(bool logErrors)
 {
   pdns::ResolveTask task;
   {
     auto lock = s_taskQueue.lock();
     if (lock->queue.empty()) {
-      return;
+      return false;
     }
     task = lock->queue.pop();
   }
@@ -236,6 +228,7 @@ void runTaskOnce(bool logErrors)
   if (expired) {
     s_taskQueue.lock()->queue.incExpired();
   }
+  return true;
 }
 
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline)
diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh
index cb9e71bf85..868b33e0af 100644
--- a/pdns/recursordist/rec-taskqueue.hh
+++ b/pdns/recursordist/rec-taskqueue.hh
@@ -32,7 +32,7 @@ namespace pdns
 struct ResolveTask;
 }
 void runTasks(size_t max, bool logErrors);
-void runTaskOnce(bool logErrors);
+bool runTaskOnce(bool logErrors);
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline);
 void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline);
 bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline, const DNSName& nsname);
diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh
index 92e3f176d1..40df635dfd 100644
--- a/pdns/recursordist/taskqueue.hh
+++ b/pdns/recursordist/taskqueue.hh
@@ -50,7 +50,7 @@ struct ResolveTask
 {
   DNSName d_qname;
   uint16_t d_qtype;
-  // Deadline is not part of index and <
+  // Deadline is not part of index and not used by operator<()
   time_t d_deadline;
   // Whether to run this task in regular mode (false) or in the mode that refreshes almost expired tasks
   bool d_refreshMode;
@@ -59,7 +59,7 @@ struct ResolveTask
   TaskFunction d_func;
   // IP used by DoT probe tasks
   ComboAddress d_ip;
-  // NS name used by DoT probe task
+  // NS name used by DoT probe task, not part of index and not used by operator<()
   DNSName d_nsname;
 
   bool operator<(const ResolveTask& a) const