From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 6 Jan 2023 11:57:04 +0000 (+0100)
Subject: Be more eager to evict expired entries in the first loop by continuing scanning the... 
X-Git-Tag: dnsdist-1.8.0-rc1~137^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ad322ced452389ea8d37354712b4a21e0bbcf8d3;p=thirdparty%2Fpdns.git

Be more eager to evict expired entries in the first loop by continuing scanning the fraction of each shard we originally computed
---

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index dc6e0f0f7e..7e47c99da2 100644
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -156,7 +156,6 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
     auto shard = content.lock();
     const auto shardSize = shard->d_map.size();
     const uint64_t toScanForThisShard = std::ceil(lookAt * ((1.0 * shardSize) / cacheSize));
-    const uint64_t toTrimForThisShard = toTrim > 0 ? std::ceil(toTrim * ((1.0 * shardSize) / cacheSize)) : 0;
     shard->invalidate();
     auto& sidx = boost::multi_index::get<S>(shard->d_map);
     uint64_t erased = 0;
@@ -172,10 +171,6 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
         ++i;
       }
 
-      if (toTrim > 0 && erased >= toTrimForThisShard) {
-        break;
-      }
-
       if (lookedAt >= toScanForThisShard) {
         break;
       }
@@ -192,11 +187,11 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
   // It was not enough, so we need to remove entries that are not
   // expired, still using the LRU index.
 
-  // From here on cacheSize is the total size of the shards that still
-  // need to be cleaned. When a shard is processed, we subtract its
-  // original size from cacheSize as it used to compute the
-  // fraction of the next shards to clean. This way rounding issues do
-  // not cause over or undershoot of the target.
+  // From here on cacheSize is the total number of entries in the
+  // shards that still need to be cleaned. When a shard is processed,
+  // we subtract its original size from cacheSize as we use this value
+  // to compute the fraction of the next shards to clean. This way
+  // rounding issues do not cause over or undershoot of the target.
   //
   // Suppose we have 10 perfectly balanced shards, each filled with
   // 100 entries. So cacheSize is 1000. When cleaning 10%, after shard
@@ -206,7 +201,7 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
   // shard, we would end up with cacheSize 100, and to clean 10.
   //
   // When the balance is not perfect, e.g. shard 0 has 54 entries, we
-  // would clean 5 entries due to rounding, and for the remaning
+  // would clean 5 entries due to rounding, and for the remaining
   // shards we start with cacheSize 946 and toTrim 95: the fraction
   // becomes slightly larger than 10%, since we "missed" one item in
   // shard 0.
diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 72977ca7ab..5e343384f8 100644
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -518,10 +518,11 @@ BOOST_AUTO_TEST_CASE(test_RecursorCache_ExpungingExpiredEntries)
   records.clear();
   BOOST_CHECK_EQUAL(MRC.size(), 2U);
 
-  /* the one for power2 having been inserted
-     more recently should be removed last */
-  /* we ask that only entry remains in the cache */
-  MRC.doPrune(1);
+  /* the one for power2 having been inserted more recently should be removed last */
+  /* we ask that 10 entries remain in the cache, this is larger than
+     the cache size (2), so 1 entry will be looked at as the code
+     rounds up the 10% of entries per shard to look at */
+  MRC.doPrune(10);
   BOOST_CHECK_EQUAL(MRC.size(), 1U);
 
   /* the remaining entry should be power2, but to get it
@@ -551,8 +552,10 @@ BOOST_AUTO_TEST_CASE(test_RecursorCache_ExpungingExpiredEntries)
 
   /* power2 should have been moved to the front of the expunge
      queue, and should this time be removed first */
-  /* we ask that only entry remains in the cache */
-  MRC.doPrune(1);
+  /* we ask that 10 entries remain in the cache, this is larger than
+     the cache size (2), so 1 entry will be looked at as the code
+     rounds up the 10% of entries per shard to look at */
+  MRC.doPrune(10);
   BOOST_CHECK_EQUAL(MRC.size(), 1U);
 
   /* the remaining entry should be power1, but to get it