From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Thu, 17 Feb 2022 14:43:55 +0000 (+0100)
Subject: Don't rate limit refresh tasks, they are already rate limited by packet and record... 
X-Git-Tag: rec-4.7.0-alpha1~10^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1bdb41ee140ef6d5091c5d20eb3546f57961ad07;p=thirdparty%2Fpdns.git

Don't rate limit refresh tasks, they are already rate limited by packet and record cache code.
---

diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc
index f59a350515..99a50fb536 100644
--- a/pdns/recursordist/rec-taskqueue.cc
+++ b/pdns/recursordist/rec-taskqueue.cc
@@ -167,22 +167,18 @@ void runTaskOnce(bool logErrors)
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline)
 {
   pdns::ResolveTask task{qname, qtype, deadline, true, resolve};
-  auto lock = s_taskQueue.lock();
-  bool running = !lock->rateLimitSet.insert(time(nullptr), task);
-  if (!running) {
-    ++s_almost_expired_tasks.pushed;
-    lock->queue.push(std::move(task));
-  }
+  s_taskQueue.lock()->queue.push(std::move(task));
+  ++s_almost_expired_tasks.pushed;
 }
 
-void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline)
+void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline)
 {
   pdns::ResolveTask task{qname, qtype, deadline, false, resolve};
   auto lock = s_taskQueue.lock();
-  bool running = !lock->rateLimitSet.insert(time(nullptr), task);
-  if (!running) {
-    ++s_resolve_tasks.pushed;
+  bool inserted = lock->rateLimitSet.insert(now, task);
+  if (inserted) {
     lock->queue.push(std::move(task));
+    ++s_resolve_tasks.pushed;
   }
 }
 
diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh
index b33e2abc51..c1e664a4af 100644
--- a/pdns/recursordist/rec-taskqueue.hh
+++ b/pdns/recursordist/rec-taskqueue.hh
@@ -28,7 +28,7 @@ class DNSName;
 
 void runTaskOnce(bool logErrors);
 void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline);
-void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline);
+void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline);
 
 // General task stats
 uint64_t getTaskPushes();
diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh
index abdd768439..b677fab30c 100644
--- a/pdns/recursordist/taskqueue.hh
+++ b/pdns/recursordist/taskqueue.hh
@@ -45,7 +45,7 @@ struct ResolveTask
   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
-  // Use a function ponter as comparing std::functions is a nuisance
+  // Use a function pointer as comparing std::functions is a nuisance
   void (*d_func)(const struct timeval& now, bool logErrors, const ResolveTask& task);
 
   bool operator<(const ResolveTask& a) const
diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 6cc8900372..6ead553219 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -559,6 +559,6 @@ void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline
   g_test_tasks.push({qname, qtype, deadline, true, nullptr});
 }
 
-void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline)
+void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline)
 {
 }
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 11973c56e4..54f45b1378 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1177,7 +1177,7 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,
       }
       if (s_doIPv6) { // s_doIPv6 **IMPLIES** pdns::isQueryLocalAddressFamilyEnabled(AF_INET6) returned true
         if (ret.empty()) {
-          // We only go out imediately to find IPv6 records if we did not find any IPv4 ones.
+          // We only go out immediately to find IPv6 records if we did not find any IPv4 ones.
           newState = vState::Indeterminate;
           cset.clear();
           if (doResolve(qname, QType::AAAA, cset, depth+1, beenthere, newState) == 0) {  // this consults cache, OR goes out
@@ -1205,18 +1205,18 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,
       }
     }
     if (s_doIPv6 && !seenV6 && !cacheOnly) {
-      // No IPv6 NS records in cache, chek negcache and submit async task if negache does not have the data
+      // No IPv6 records in cache, check negcache and submit async task if negache does not have the data
       // so that the next time the cache or the negcache will have data
       NegCache::NegCacheEntry ne;
       bool inNegCache = g_negCache->get(qname, QType::AAAA, d_now, ne, true);
       if (!inNegCache) {
-        pushResolveTask(qname, QType::AAAA, d_now.tv_sec + 60);
+        pushResolveTask(qname, QType::AAAA, d_now.tv_sec, d_now.tv_sec + 60);
       }
     }
   }
-  catch (const PolicyHitException& e) {
-    /* we ignore a policy hit while trying to retrieve the addresses
-       of a NS and keep processing the current query */
+  catch (const PolicyHitException&) {
+    // We ignore a policy hit while trying to retrieve the addresses
+    // of a NS and keep processing the current query
   }
 
   if (ret.empty() && d_outqueries > startqueries) {