From: Otto Moerbeek Date: Sun, 6 Feb 2022 14:38:48 +0000 (+0100) Subject: Tell packet cache it's max size and use it on insert to immediately X-Git-Tag: rec-4.7.0-beta1~23^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eeab0dc2bfd93649bc4fa8f995ee9e7da9a86503;p=thirdparty%2Fpdns.git Tell packet cache it's max size and use it on insert to immediately delete the oldest entry when we're over-sized. Also, if we're seeing a stale cache item, don't move it to the front of the sequence, as we're almost always going to update it and then it will be moved again to the back. --- diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh index f3af11af77..018e2a4457 100644 --- a/pdns/cachecleaner.hh +++ b/pdns/cachecleaner.hh @@ -28,10 +28,10 @@ // this function can clean any cache that has a getTTD() method on its entries, a preRemoval() method and a 'sequence' index as its second index // the ritual is that the oldest entries are in *front* of the sequence collection, so on a hit, move an item to the end -// on a miss, move it to the beginning +// and optionally, on a miss, move it to the beginning template void pruneCollection(C& container, T& collection, size_t maxCached, size_t scanFraction = 1000) { - const time_t now = time(0); + const time_t now = time(nullptr); size_t toTrim = 0; const size_t cacheSize = collection.size(); diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc index 887ccb5824..6d9bae7494 100644 --- a/pdns/recpacketcache.cc +++ b/pdns/recpacketcache.cc @@ -10,11 +10,6 @@ #include "namespaces.hh" #include "rec-taskqueue.hh" -RecursorPacketCache::RecursorPacketCache() -{ - d_hits = d_misses = 0; -} - unsigned int RecursorPacketCache::s_refresh_ttlperc{0}; int RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype, bool subtree) @@ -97,7 +92,8 @@ bool RecursorPacketCache::checkResponseMatches(std::pair(d_packetCache, iter); + // We used to move the item to the fron ot the to be deleted sequence, + // but we're very likely will update the entry very soon, so leave it d_misses++; break; } @@ -194,14 +190,15 @@ void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, } d_packetCache.insert(e); -} -uint64_t RecursorPacketCache::size() -{ - return d_packetCache.size(); + if (d_packetCache.size() > d_maxSize) { + auto it = d_packetCache.begin(); + d_packetCache.erase(it); + } + } -uint64_t RecursorPacketCache::bytes() +uint64_t RecursorPacketCache::bytes() const { uint64_t sum = 0; for (const auto& e : d_packetCache) { diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh index d1084bbf63..28554b3472 100644 --- a/pdns/recpacketcache.hh +++ b/pdns/recpacketcache.hh @@ -59,7 +59,11 @@ public: }; typedef boost::optional OptPBData; - RecursorPacketCache(); + RecursorPacketCache(size_t maxsize) + : d_maxSize(maxsize) + { + } + bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, OptPBData* pbdata, bool tcp); @@ -70,10 +74,19 @@ public: uint64_t doDump(int fd); int doWipePacketCache(const DNSName& name, uint16_t qtype = 0xffff, bool subtree = false); - void prune(); - uint64_t d_hits, d_misses; - uint64_t size(); - uint64_t bytes(); + void setMaxSize(size_t sz) + { + d_maxSize = sz; + } + + uint64_t size() const + { + return d_packetCache.size(); + } + uint64_t bytes() const; + + uint64_t d_hits{0}; + uint64_t d_misses{0}; private: struct HashTag @@ -131,6 +144,7 @@ private: packetCache_t; packetCache_t d_packetCache; + size_t d_maxSize; static bool qrMatch(const packetCache_t::index::type::iterator& iter, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass); bool checkResponseMatches(std::pair::type::iterator, packetCache_t::index::type::iterator> range, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, OptPBData* pbdata); diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 131b9021e1..2af4fb2f1a 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1867,11 +1867,13 @@ static void houseKeeping(void*) struct timeval now; Utility::gettimeofday(&now); + // Below are the tasks that run for every recursorThread, including handler and taskThread if (t_packetCache) { - // Below are the tasks that run for every recursorThread, including handler and taskThread static thread_local PeriodicTask packetCacheTask{"packetCacheTask", 5}; packetCacheTask.runIfDue(now, []() { - t_packetCache->doPruneTo(g_maxPacketCacheEntries / (RecThreadInfo::numDistributors() + RecThreadInfo::numWorkers())); + size_t sz = g_maxPacketCacheEntries / (RecThreadInfo::numWorkers() + RecThreadInfo::numDistributors()); + t_packetCache->setMaxSize(sz); // might have changed by rec_control + t_packetCache->doPruneTo(sz); }); } @@ -2060,7 +2062,7 @@ static void recursorThread() } if (!::arg().mustDo("disable-packetcache") && (threadInfo.isDistributor() || threadInfo.isWorker())) { - t_packetCache = std::make_unique(); + t_packetCache = std::make_unique(g_maxPacketCacheEntries / (RecThreadInfo::numWorkers() + RecThreadInfo::numDistributors())); } #ifdef NOD_ENABLED diff --git a/pdns/test-recpacketcache_cc.cc b/pdns/test-recpacketcache_cc.cc index b79bf3c091..996113c5c8 100644 --- a/pdns/test-recpacketcache_cc.cc +++ b/pdns/test-recpacketcache_cc.cc @@ -17,7 +17,7 @@ BOOST_AUTO_TEST_SUITE(test_recpacketcache_cc) BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) { - RecursorPacketCache rpc; + RecursorPacketCache rpc(1000); string fpacket; unsigned int tag = 0; uint32_t age = 0; @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) BOOST_AUTO_TEST_CASE(test_recPacketCacheSimplePost2038) { - RecursorPacketCache rpc; + RecursorPacketCache rpc(1000); string fpacket; unsigned int tag = 0; uint32_t age = 0; @@ -168,7 +168,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_Tags) should not override the first one, and we should get a hit for the query with either tags, with the response matching the tag. */ - RecursorPacketCache rpc; + RecursorPacketCache rpc(1000); string fpacket; const unsigned int tag1 = 0; const unsigned int tag2 = 42; @@ -287,7 +287,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_TCP) /* Insert a response with UDP, the exact same query with a TCP flag should lead to a miss. */ - RecursorPacketCache rpc; + RecursorPacketCache rpc(1000); string fpacket; uint32_t age = 0; uint32_t qhash = 0;