From: Otto Moerbeek Date: Fri, 1 Sep 2023 13:39:18 +0000 (+0200) Subject: rec: implement a more fair way to prune the aggressive cache X-Git-Tag: rec-4.9.2~8^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a0c0d07b48abda4d1ecfaf167978b9ba7573eaef;p=thirdparty%2Fpdns.git rec: implement a more fair way to prune the aggressive cache Fixes #13109 (cherry picked from commit f44081141772da42dd6830462ae357530d3a1fbf) --- diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc index e17bf2dd3a..22fe68aad8 100644 --- a/pdns/recursordist/aggressive_nsec.cc +++ b/pdns/recursordist/aggressive_nsec.cc @@ -126,76 +126,72 @@ void AggressiveNSECCache::prune(time_t now) { uint64_t maxNumberOfEntries = d_maxEntries; std::vector emptyEntries; - uint64_t erased = 0; - uint64_t lookedAt = 0; - uint64_t toLook = std::max(d_entriesCount / 5U, static_cast(1U)); - if (d_entriesCount > maxNumberOfEntries) { - uint64_t toErase = d_entriesCount - maxNumberOfEntries; - toLook = toErase * 5; - // we are full, scan at max 5 * toErase entries and stop once we have nuked enough + auto zones = d_zones.write_lock(); + // To start, just look through 10% of each zone and nuke everything that is expired + zones->visit([now, &erased, &emptyEntries](const SuffixMatchTree>>& node) { + if (!node.d_value) { + return; + } - auto zones = d_zones.write_lock(); - zones->visit([now, &erased, toErase, toLook, &lookedAt, &emptyEntries](const SuffixMatchTree>>& node) { - if (!node.d_value || erased > toErase || lookedAt > toLook) { - return; + auto zoneEntry = node.d_value->lock(); + auto& sidx = boost::multi_index::get(zoneEntry->d_entries); + const auto toLookAtForThisZone = (zoneEntry->d_entries.size() + 9) / 10; + uint64_t lookedAt = 0; + for (auto it = sidx.begin(); it != sidx.end() && lookedAt < toLookAtForThisZone; ++lookedAt) { + if (it->d_ttd < now) { + it = sidx.erase(it); + ++erased; + } + else { + ++it; } + } - auto zoneEntry = node.d_value->lock(); - auto& sidx = boost::multi_index::get(zoneEntry->d_entries); - for (auto it = sidx.begin(); it != sidx.end(); ++lookedAt) { - if (erased >= toErase || lookedAt >= toLook) { - break; - } + if (zoneEntry->d_entries.empty()) { + emptyEntries.push_back(zoneEntry->d_zone); + } + }); - if (it->d_ttd < now) { - it = sidx.erase(it); - ++erased; - } - else { - ++it; - } - } + d_entriesCount -= erased; - if (zoneEntry->d_entries.size() == 0) { - emptyEntries.push_back(zoneEntry->d_zone); - } - }); - } - else { - // we are not full, just look through 10% of the cache and nuke everything that is expired - auto zones = d_zones.write_lock(); - zones->visit([now, &erased, toLook, &lookedAt, &emptyEntries](const SuffixMatchTree>>& node) { - if (!node.d_value) { + // If we are still above try harder by nuking entries from each zone in LRU order + auto entriesCount = d_entriesCount.load(); + if (entriesCount > maxNumberOfEntries) { + erased = 0; + uint64_t toErase = entriesCount - maxNumberOfEntries; + zones->visit([&erased, &toErase, &entriesCount, &emptyEntries](const SuffixMatchTree>>& node) { + if (!node.d_value || entriesCount == 0) { return; } - auto zoneEntry = node.d_value->lock(); + const auto zoneSize = zoneEntry->d_entries.size(); auto& sidx = boost::multi_index::get(zoneEntry->d_entries); - for (auto it = sidx.begin(); it != sidx.end(); ++lookedAt) { - if (lookedAt >= toLook) { + const auto toTrimForThisZone = static_cast(std::round(static_cast(toErase) * static_cast(zoneSize) / static_cast(entriesCount))); + if (entriesCount < zoneSize) { + throw std::runtime_error("Inconsistent agggressive cache " + std::to_string(entriesCount) + " " + std::to_string(zoneSize)); + } + // This is comparable to what cachecleaner.hh::pruneMutexCollectionsVector() is doing, look there for an explanation + entriesCount -= zoneSize; + uint64_t trimmedFromThisZone = 0; + for (auto it = sidx.begin(); it != sidx.end() && trimmedFromThisZone < toTrimForThisZone; ) { + it = sidx.erase(it); + ++erased; + ++trimmedFromThisZone; + if (--toErase == 0) { break; } - if (it->d_ttd < now || lookedAt > toLook) { - it = sidx.erase(it); - ++erased; - } - else { - ++it; - } } - - if (zoneEntry->d_entries.size() == 0) { + if (zoneEntry->d_entries.empty()) { emptyEntries.push_back(zoneEntry->d_zone); } }); - } - d_entriesCount -= erased; + d_entriesCount -= erased; + } if (!emptyEntries.empty()) { - auto zones = d_zones.write_lock(); for (const auto& entry : emptyEntries) { zones->remove(entry); } @@ -399,13 +395,14 @@ bool AggressiveNSECCache::getNSECBefore(time_t now, std::shared_ptrd_entries.project(it); if (it->d_ttd <= now) { - moveCacheItemToFront(zoneEntry->d_entries, it); + moveCacheItemToFront(zoneEntry->d_entries, firstIndexIterator); return false; } entry = *it; - moveCacheItemToBack(zoneEntry->d_entries, it); + moveCacheItemToBack(zoneEntry->d_entries, firstIndexIterator); return true; } diff --git a/pdns/recursordist/test-aggressive_nsec_cc.cc b/pdns/recursordist/test-aggressive_nsec_cc.cc index 8222a812a2..879f81b9a3 100644 --- a/pdns/recursordist/test-aggressive_nsec_cc.cc +++ b/pdns/recursordist/test-aggressive_nsec_cc.cc @@ -1076,6 +1076,52 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec3_wildcard_synthesis) BOOST_CHECK_EQUAL(queriesCount, 5U); } +BOOST_AUTO_TEST_CASE(test_aggressive_nsec_replace) +{ + const size_t testSize = 10000; + auto cache = make_unique(testSize); + + struct timeval now{}; + Utility::gettimeofday(&now, nullptr); + + vector names; + names.reserve(testSize); + for (size_t i = 0; i < testSize; i++) { + names.emplace_back(std::to_string(i) + "powerdns.com"); + } + + DTime time; + time.set(); + + for (const auto& name : names) { + DNSRecord rec; + rec.d_name = name; + rec.d_type = QType::NSEC3; + rec.d_ttl = now.tv_sec + 10; + rec.setContent(getRecordContent(QType::NSEC3, "1 0 500 ab HASG==== A RRSIG NSEC3")); + auto rrsig = std::make_shared("NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, {rrsig}, true); + } + auto diff1 = time.udiff(true); + + BOOST_CHECK_EQUAL(cache->getEntriesCount(), testSize); + for (const auto& name : names) { + DNSRecord rec; + rec.d_name = name; + rec.d_type = QType::NSEC3; + rec.d_ttl = now.tv_sec + 10; + rec.setContent(getRecordContent(QType::NSEC3, "1 0 500 ab HASG==== A RRSIG NSEC3")); + auto rrsig = std::make_shared("NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, {rrsig}, true); + } + + BOOST_CHECK_EQUAL(cache->getEntriesCount(), testSize); + + auto diff2 = time.udiff(true); + // Check that replace is about equally fast as insert + BOOST_ASSERT(diff1 < diff2 * 1.2 && diff2 < diff1 * 1.2); +} + BOOST_AUTO_TEST_CASE(test_aggressive_nsec_wiping) { auto cache = make_unique(10000); @@ -1152,7 +1198,7 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec_pruning) rec.d_name = DNSName("www.powerdns.org"); rec.d_type = QType::NSEC3; - rec.d_ttl = now.tv_sec + 10; + rec.d_ttl = now.tv_sec + 20; rec.setContent(getRecordContent(QType::NSEC3, "1 0 500 ab HASG==== A RRSIG NSEC3")); rrsig = std::make_shared("NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data"); cache->insertNSEC(DNSName("powerdns.org"), rec.d_name, rec, {rrsig}, true); @@ -1160,18 +1206,13 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec_pruning) BOOST_CHECK_EQUAL(cache->getEntriesCount(), 3U); /* we have set a upper bound to 2 entries, so we are above, - and all entries are actually expired, so we will prune one entry + and one entry are actually expired, so we will prune one entry to get below the limit */ - cache->prune(now.tv_sec + 600); + cache->prune(now.tv_sec + 15); BOOST_CHECK_EQUAL(cache->getEntriesCount(), 2U); - /* now we are at the limit, so we will scan 1/5th of the entries, - and prune the expired ones, which mean we will also remove only one */ - cache->prune(now.tv_sec + 600); - BOOST_CHECK_EQUAL(cache->getEntriesCount(), 1U); - - /* now we are below the limit, so we will scan 1/5th of the entries again, - and prune the expired ones, which mean we will remove the last one */ + /* now we are at the limit, so we will scan 1/5th of all zones entries, rounded up, + and prune the expired ones, which mean we will also remoing twoe */ cache->prune(now.tv_sec + 600); BOOST_CHECK_EQUAL(cache->getEntriesCount(), 0U); }