From: Remi Gacogne Date: Fri, 11 Mar 2022 15:27:56 +0000 (+0100) Subject: dnsdist: Defer the actual allocation of the ring buffer entries X-Git-Tag: auth-4.7.0-alpha1~65^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ed3021e6eb93ecbe68d68955405d712197d7a36;p=thirdparty%2Fpdns.git 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 f0c3caacac..fe35307922 100644 --- 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->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 s; diff --git a/pdns/dnsdist-rings.hh b/pdns/dnsdist-rings.hh index 6ee3f5d562..86591add65 100644 --- a/pdns/dnsdist-rings.hh +++ b/pdns/dnsdist-rings.hh @@ -69,43 +69,19 @@ struct Rings { LockGuarded> respRing{boost::circular_buffer()}; }; - 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 > > 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->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 d_nbQueryEntries; std::atomic d_nbResponseEntries; std::atomic d_currentShardId; + std::atomic 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 31fc9ad247..bd0984df82 100644 --- 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 61b5ffb3ec..bfb9c87e84 100644 --- 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 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 18f6d04f83..8fa8dbf2e7 100644 --- 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