dnsdist: Defer the actual allocation of the ring buffer entries

It is a bit cumbersome to have to call `Rings::init()` when we are
ready, but it prevents allocating the default number of entries,
10k, which is quite a lot for small setups. Of course the memory
was released once the configuration had been parsed, but it might
be too late in some cases, and we would end up with a bigger reported
memory usage than our actual one since heap shrinkage seldom happens.

diff --git a/pdns/dnsdist-rings.cc b/pdns/dnsdist-rings.cc
index f0c3caacac466e9380a509be8d3db0d5f69d58b4..fe3530792222f07ad7a14e33b4da139939f185a9 100644 (file)
--- a/pdns/dnsdist-rings.cc
+++ b/pdns/dnsdist-rings.cc
@@ -24,6 +24,49 @@
 
 #include "dnsdist-rings.hh"
 
+void Rings::setCapacity(size_t newCapacity, size_t numberOfShards)
+{
+  if (d_initialized) {
+    throw std::runtime_error("Rings::setCapacity() should not be called once the rings have been initialized");
+  }
+  d_capacity = newCapacity;
+  d_numberOfShards = numberOfShards;
+}
+
+void Rings::init()
+{
+  if (d_initialized) {
+    throw std::runtime_error("Rings::init() should only be called once");
+  }
+  d_initialized = true;
+
+  if (d_numberOfShards <= 1) {
+    d_nbLockTries = 0;
+  }
+
+  d_shards.resize(d_numberOfShards);
+
+  /* resize all the rings */
+  for (auto& shard : d_shards) {
+    shard = std::make_unique<Shard>();
+    shard->queryRing.lock()->set_capacity(d_capacity / d_numberOfShards);
+    shard->respRing.lock()->set_capacity(d_capacity / d_numberOfShards);
+  }
+
+  /* we just recreated the shards so they are now empty */
+  d_nbQueryEntries = 0;
+  d_nbResponseEntries = 0;
+}
+
+void Rings::setNumberOfLockRetries(size_t retries)
+{
+  if (d_numberOfShards <= 1) {
+    d_nbLockTries = 0;
+  } else {
+    d_nbLockTries = retries;
+  }
+}
+
 size_t Rings::numDistinctRequestors()
 {
   std::set<ComboAddress, ComboAddress::addressOnlyLessThan> s;

diff --git a/pdns/dnsdist-rings.hh b/pdns/dnsdist-rings.hh
index 6ee3f5d562cb2504a8b0efb5db0515e512fff25b..86591add65776dd0bba15616ce7f10edf24413ff 100644 (file)
--- a/pdns/dnsdist-rings.hh
+++ b/pdns/dnsdist-rings.hh
@@ -69,43 +69,19 @@ struct Rings {
     LockGuarded<boost::circular_buffer<Response>> respRing{boost::circular_buffer<Response>()};
   };
 
-  Rings(size_t capacity=10000, size_t numberOfShards=10, size_t nbLockTries=5, bool keepLockingStats=false): d_blockingQueryInserts(0), d_blockingResponseInserts(0), d_deferredQueryInserts(0), d_deferredResponseInserts(0), d_nbQueryEntries(0), d_nbResponseEntries(0), d_currentShardId(0), d_numberOfShards(numberOfShards), d_nbLockTries(nbLockTries), d_keepLockingStats(keepLockingStats)
+  Rings(size_t capacity=10000, size_t numberOfShards=10, size_t nbLockTries=5, bool keepLockingStats=false): d_blockingQueryInserts(0), d_blockingResponseInserts(0), d_deferredQueryInserts(0), d_deferredResponseInserts(0), d_nbQueryEntries(0), d_nbResponseEntries(0), d_currentShardId(0), d_capacity(capacity), d_numberOfShards(numberOfShards), d_nbLockTries(nbLockTries), d_keepLockingStats(keepLockingStats)
   {
-    setCapacity(capacity, numberOfShards);
   }
 
   std::unordered_map<int, vector<boost::variant<string,double> > > getTopBandwidth(unsigned int numentries);
   size_t numDistinctRequestors();
-  /* This function should only be called at configuration time before any query or response has been inserted */
-  void setCapacity(size_t newCapacity, size_t numberOfShards)
-  {
-    if (numberOfShards <= 1) {
-      d_nbLockTries = 0;
-    }
-
-    d_shards.resize(numberOfShards);
-    d_numberOfShards = numberOfShards;
+  /* this function should not be called after init() has been called */
+  void setCapacity(size_t newCapacity, size_t numberOfShards);
 
-    /* resize all the rings */
-    for (auto& shard : d_shards) {
-      shard = std::make_unique<Shard>();
-      shard->queryRing.lock()->set_capacity(newCapacity / numberOfShards);
-      shard->respRing.lock()->set_capacity(newCapacity / numberOfShards);
-    }
-
-    /* we just recreated the shards so they are now empty */
-    d_nbQueryEntries = 0;
-    d_nbResponseEntries = 0;
-  }
+  /* This function should only be called at configuration time before any query or response has been inserted */
+  void init();
 
-  void setNumberOfLockRetries(size_t retries)
-  {
-    if (d_numberOfShards <= 1) {
-      d_nbLockTries = 0;
-    } else {
-      d_nbLockTries = retries;
-    }
-  }
+  void setNumberOfLockRetries(size_t retries);
 
   size_t getNumberOfShards() const
   {
@@ -199,6 +175,13 @@ struct Rings {
     d_deferredResponseInserts.store(0);
   }
 
+  /* this should be called in the unit tests, and never at runtime */
+  void reset()
+  {
+    clear();
+    d_initialized = false;
+  }
+
   /* load the content of the ring buffer from a file in the format emitted by grepq(),
      only useful for debugging purposes */
   size_t loadFromFile(const std::string& filepath, const struct timespec& now);
@@ -251,7 +234,9 @@ private:
   std::atomic<size_t> d_nbQueryEntries;
   std::atomic<size_t> d_nbResponseEntries;
   std::atomic<size_t> d_currentShardId;
+  std::atomic<bool> d_initialized{false};
 
+  size_t d_capacity;
   size_t d_numberOfShards;
   size_t d_nbLockTries = 5;
   bool d_keepLockingStats{false};

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 31fc9ad247fee999b5d41ab3bf83472750262bfa..bd0984df824b2d46ac45cf27d5d6673e9988c4b2 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -2543,6 +2543,8 @@ int main(int argc, char** argv)
 
     g_configurationDone = true;
 
+    g_rings.init();
+
     for(auto& frontend : g_frontends) {
       setUpLocalBind(frontend);
 

diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
index 61b5ffb3ec97d5f71ce37b5d667a1653c4c40976..bfb9c87e847a400a833cd73fa0b2c4ceb066be55 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
+++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
@@ -36,6 +36,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_QueryRate) {
   const auto action = DNSAction::Action::Drop;
   const std::string reason = "Exceeded query rate";
 
+  g_rings.reset();
+  g_rings.init();
+
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
 
@@ -177,6 +180,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_QueryRate_RangeV6) {
   dbrg.setQuiet(true);
   dbrg.setMasks(32, 64, 0);
 
+  g_rings.reset();
+  g_rings.init();
+
   /* block above 50 qps for numberOfSeconds seconds, no warning */
   dbrg.setQueryRate(50, 0, numberOfSeconds, reason, blockDuration, action);
 
@@ -277,6 +283,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_QueryRate_V4Ports) {
   /* split v4 by ports using a  /2 (0 - 16383, 16384 - 32767, 32768 - 49151, 49152 - 65535) */
   dbrg.setMasks(32, 128, 2);
 
+  g_rings.reset();
+  g_rings.init();
+
   /* block above 50 qps for numberOfSeconds seconds, no warning */
   dbrg.setQueryRate(50, 0, numberOfSeconds, reason, blockDuration, action);
 
@@ -401,16 +410,15 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_QueryRate_responses) {
   NetmaskTree<DynBlock, AddressAndPortRange> emptyNMG;
 
   /* 100k entries, one shard */
+  g_rings.reset();
   g_rings.setCapacity(1000000, 1);
+  g_rings.init();
 
   size_t numberOfSeconds = 10;
   size_t blockDuration = 60;
   const auto action = DNSAction::Action::Drop;
   const std::string reason = "Exceeded query rate";
 
-  /* 100k entries, one shard */
-  g_rings.setCapacity(1000000, 1);
-
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
 
@@ -465,6 +473,8 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_QTypeRate) {
 
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
+  g_rings.reset();
+  g_rings.init();
 
   /* block above 50 qps for numberOfSeconds seconds, no warning */
   dbrg.setQTypeRate(QType::AAAA, 50, 0, numberOfSeconds, reason, blockDuration, action);
@@ -650,6 +660,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_RCodeRatio) {
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
 
+  g_rings.reset();
+  g_rings.init();
+
   /* block above 0.2 ServFail/Total ratio over numberOfSeconds seconds, no warning, minimum number of queries should be at least 51 */
   dbrg.setRCodeRatio(rcode, 0.2, 0, numberOfSeconds, reason, blockDuration, action, 51);
 
@@ -769,6 +782,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_ResponseByteRate) {
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
 
+  g_rings.reset();
+  g_rings.init();
+
   /* block above 10kB/s for numberOfSeconds seconds, no warning */
   dbrg.setResponseByteRate(10000, 0, numberOfSeconds, reason, blockDuration, action);
 
@@ -840,6 +856,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_Warning) {
   DynBlockRulesGroup dbrg;
   dbrg.setQuiet(true);
 
+  g_rings.reset();
+  g_rings.init();
+
   /* warn above 20 qps for numberOfSeconds seconds, block above 50 qps */
   dbrg.setQueryRate(50, 20, numberOfSeconds, reason, blockDuration, action);
 
@@ -1007,6 +1026,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesGroup_Ranges) {
   /* block above 50 qps for numberOfSeconds seconds, no warning */
   dbrg.setQueryRate(50, 0, numberOfSeconds, reason, blockDuration, action);
 
+  g_rings.reset();
+  g_rings.init();
+
   {
     /* insert just above 50 qps from the two clients in the last 10s
        this should trigger the rule for the first one but not the second one */
@@ -1054,8 +1076,10 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) {
   const auto action = DNSAction::Action::Drop;
   const std::string reason = "Exceeded query rate";
 
+  g_rings.reset();
   /* 10M entries, only one shard */
   g_rings.setCapacity(10000000, 1);
+  g_rings.init();
 
   {
     DynBlockRulesGroup dbrg;

diff --git a/pdns/dnsdistdist/test-dnsdistrings_cc.cc b/pdns/dnsdistdist/test-dnsdistrings_cc.cc
index 18f6d04f83847e2c7d7b8a4c7a305a3a01cd4288..8fa8dbf2e71942ed26a53487c1f02160104accf7 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistrings_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistrings_cc.cc
@@ -13,6 +13,7 @@ BOOST_AUTO_TEST_SUITE(dnsdistrings_cc)
 static void test_ring(size_t maxEntries, size_t numberOfShards, size_t nbLockTries)
 {
   Rings rings(maxEntries, numberOfShards, nbLockTries);
+  rings.init();
   size_t entriesPerShard = maxEntries / numberOfShards;
 
   BOOST_CHECK_EQUAL(rings.getNumberOfShards(), numberOfShards);
@@ -205,6 +206,7 @@ BOOST_AUTO_TEST_CASE(test_Rings_Threaded) {
   dnsdist::Protocol outgoingProtocol = dnsdist::Protocol::DoUDP;
 
   Rings rings(numberOfEntries, numberOfShards, lockAttempts, true);
+  rings.init();
 #if defined(DNSDIST_RINGS_WITH_MACADDRESS)
   Rings::Query query({requestor, qname, now, dh, size, qtype, protocol, "", false});
 #else