dnsdist: Use separate sampling counters for queries and responses

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsdistdist/dnsdist-rings.cc b/pdns/dnsdistdist/dnsdist-rings.cc
index 8ac89cb78633792c5af470d8c5db1c6f62733eba..9f1231c30e6e72185d4cf455732e9f463219c645 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-rings.cc
+++ b/pdns/dnsdistdist/dnsdist-rings.cc
@@ -24,7 +24,8 @@
 
 #include "dnsdist-rings.hh"
 
-thread_local size_t Rings::t_samplingCounter{0};
+thread_local size_t Rings::t_samplingQueryCounter{0};
+thread_local size_t Rings::t_samplingResponseCounter{0};
 
 void Rings::init(const RingsConfiguration& config)
 {
@@ -204,12 +205,21 @@ bool Rings::Response::isACacheHit() const
   return hit;
 }
 
-bool Rings::shouldSkipDueToSampling()
+bool Rings::shouldSkipQueryDueToSampling()
 {
   if (d_samplingRate == 0) {
     return false;
   }
-  auto counter = t_samplingCounter++;
+  auto counter = t_samplingQueryCounter++;
+  return (counter % d_samplingRate) != 0;
+}
+
+bool Rings::shouldSkipResponseDueToSampling()
+{
+  if (d_samplingRate == 0) {
+    return false;
+  }
+  auto counter = t_samplingResponseCounter++;
   return (counter % d_samplingRate) != 0;
 }
 

diff --git a/pdns/dnsdistdist/dnsdist-rings.hh b/pdns/dnsdistdist/dnsdist-rings.hh
index ef99dd2c7da1a2bb5c0b0c0b3233ceb974b3058b..a07a04a1583b02576bc8d7bf8d2bffaf2fea5d4a 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-rings.hh
+++ b/pdns/dnsdistdist/dnsdist-rings.hh
@@ -106,7 +106,7 @@ struct Rings
 
   void insertQuery(const struct timespec& when, const ComboAddress& requestor, const DNSName& name, uint16_t qtype, uint16_t size, const struct dnsheader& dh, dnsdist::Protocol protocol)
   {
-    if (shouldSkipDueToSampling()) {
+    if (shouldSkipQueryDueToSampling()) {
       return;
     }
     auto ourName = DNSName(name);
@@ -162,7 +162,7 @@ struct Rings
 
   void insertResponse(const struct timespec& when, const ComboAddress& requestor, const DNSName& name, uint16_t qtype, unsigned int usec, unsigned int size, const struct dnsheader& dh, const ComboAddress& backend, dnsdist::Protocol protocol)
   {
-    if (shouldSkipDueToSampling()) {
+    if (shouldSkipResponseDueToSampling()) {
       return;
     }
     auto ourName = DNSName(name);
@@ -221,6 +221,9 @@ struct Rings
   {
     clear();
     d_initialized = false;
+    /* this will only clear the counter for the current thread! */
+    t_samplingQueryCounter = 0;
+    t_samplingResponseCounter = 0;
   }
 
   /* load the content of the ring buffer from a file in the format emitted by grepq(),
@@ -287,12 +290,15 @@ private:
     return wasFull;
   }
 
-  bool shouldSkipDueToSampling();
+  bool shouldSkipQueryDueToSampling();
+  bool shouldSkipResponseDueToSampling();
 
   static constexpr bool s_keepLockingStats{false};
   // small hack to reduce contention: this only works because we have a single Rings object in DNSdist
-  static thread_local size_t t_samplingCounter;
-
+  // we keep separate counters for queries or responses, otherwise there is an actual
+  // risk of skipping only one of these
+  static thread_local size_t t_samplingQueryCounter;
+  static thread_local size_t t_samplingResponseCounter;
 
   std::atomic<size_t> d_nbQueryEntries{0};
   std::atomic<size_t> d_nbResponseEntries{0};