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 f3af11af7765aa4a466f394cbf59fb37fe57da84..018e2a4457d8c9bef6fa3bc4c132d73a51772106 100644 (file)
--- 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 <typename S, typename C, typename T> 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 887ccb582448685a91b1e2fbade1ab00954befff..6d9bae7494e50952044f0d0cac43de804483f057 100644 (file)
--- 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<packetCache_t::index<Ha
       return true;
     }
     else {
-      moveCacheItemToFront<SequencedTag>(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 d1084bbf6336f981086ffde693f56109db94df18..28554b34722eab20abc796e0151ec8d9c66a8ad6 100644 (file)
--- a/pdns/recpacketcache.hh
+++ b/pdns/recpacketcache.hh
@@ -59,7 +59,11 @@ public:
   };
   typedef boost::optional<PBData> 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<HashTag>::type::iterator& iter, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass);
   bool checkResponseMatches(std::pair<packetCache_t::index<HashTag>::type::iterator, packetCache_t::index<HashTag>::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 131b9021e12f2780c9f1824d706905dc7a64b5eb..2af4fb2f1a1629f578400e3412c4e1f1ebbee3c1 100644 (file)
--- 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<RecursorPacketCache>();
+      t_packetCache = std::make_unique<RecursorPacketCache>(g_maxPacketCacheEntries / (RecThreadInfo::numWorkers() + RecThreadInfo::numDistributors()));
     }
 
 #ifdef NOD_ENABLED

diff --git a/pdns/test-recpacketcache_cc.cc b/pdns/test-recpacketcache_cc.cc
index b79bf3c09113a620f835a54e5a33e408404e3c20..996113c5c84fe72fb9953f1c4cbd65401f2f39d5 100644 (file)
--- 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;