From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 25 Aug 2021 07:57:10 +0000 (+0200)
Subject: rec: Fix the acquired/contended cache counters not being updated
X-Git-Tag: dnsdist-1.7.0-alpha1~53^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bbad1c01a5c1628d151917d9485414ff47dbe7b2;p=thirdparty%2Fpdns.git

rec: Fix the acquired/contended cache counters not being updated
---

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index 55ddd48fc1..fd164fa697 100644
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -144,7 +144,7 @@ template <typename S, typename C, typename T> uint64_t pruneMutexCollectionsVect
   }
 
   for (auto& content : maps) {
-    auto mc = content.d_content.lock();
+    auto mc = content.lock();
     mc->invalidate();
     auto& sidx = boost::multi_index::get<S>(mc->d_map);
     uint64_t erased = 0, lookedAt = 0;
@@ -178,7 +178,7 @@ template <typename S, typename C, typename T> uint64_t pruneMutexCollectionsVect
   while (true) {
     size_t pershard = toTrim / maps_size + 1;
     for (auto& content : maps) {
-      auto mc = content.d_content.lock();
+      auto mc = content.lock();
       mc->invalidate();
       auto& sidx = boost::multi_index::get<S>(mc->d_map);
       size_t removed = 0;
diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 26b0a75839..a630a2e496 100644
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -32,7 +32,7 @@ pair<uint64_t,uint64_t> MemRecursorCache::stats()
 {
   uint64_t c = 0, a = 0;
   for (auto& mc : d_maps) {
-    auto content = mc.d_content.lock();
+    auto content = mc.lock();
     c += content->d_contended_count;
     a += content->d_acquired_count;
   }
@@ -44,7 +44,7 @@ size_t MemRecursorCache::ecsIndexSize()
   // XXX!
   size_t count = 0;
   for (auto& mc : d_maps) {
-    auto content = mc.d_content.lock();
+    auto content = mc.lock();
     count += content->d_ecsIndex.size();
   }
   return count;
@@ -55,7 +55,7 @@ size_t MemRecursorCache::bytes()
 {
   size_t ret = 0;
   for (auto& mc : d_maps) {
-    auto m = mc.d_content.lock();
+    auto m = mc.lock();
     for (const auto& i : m->d_map) {
       ret += sizeof(struct CacheEntry);
       ret += i.d_qname.toString().length();
@@ -273,7 +273,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
   }
 
   auto& mc = getMap(qname);
-  auto map = mc.d_content.lock();
+  auto map = mc.lock();
 
   /* If we don't have any netmask-specific entries at all, let's just skip this
      to be able to use the nice d_cachecache hack. */
@@ -389,7 +389,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b
 void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, const std::vector<std::shared_ptr<DNSRecord>>& authorityRecs, bool auth, const DNSName& authZone, boost::optional<Netmask> ednsmask, const OptTag& routingTag, vState state, boost::optional<ComboAddress> from)
 {
   auto& mc = getMap(qname);
-  auto map = mc.d_content.lock();
+  auto map = mc.lock();
 
   map->d_cachecachevalid = false;
   if (ednsmask) {
@@ -492,7 +492,7 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
 
   if (!sub) {
     auto& mc = getMap(name);
-    auto map = mc.d_content.lock();
+    auto map = mc.lock();
     map->d_cachecachevalid = false;
     auto& idx = map->d_map.get<OrderedTag>();
     auto range = idx.equal_range(name);
@@ -520,7 +520,7 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
   }
   else {
     for (auto& mc : d_maps) {
-      auto map = mc.d_content.lock();
+      auto map = mc.lock();
       map->d_cachecachevalid = false;
       auto& idx = map->d_map.get<OrderedTag>();
       for (auto i = idx.lower_bound(name); i != idx.end(); ) {
@@ -553,7 +553,7 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
 bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, const QType qtype, uint32_t newTTL)
 {
   auto& mc = getMap(name);
-  auto map = mc.d_content.lock();
+  auto map = mc.lock();
   cache_t::iterator iter = map->d_map.find(tie(name, qtype));
   if (iter == map->d_map.end()) {
     return false;
@@ -589,7 +589,7 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
   }
 
   auto& mc = getMap(qname);
-  auto map = mc.d_content.lock();
+  auto map = mc.lock();
 
   bool updated = false;
   if (!map->d_ecsIndex.empty() && !routingTag) {
@@ -642,7 +642,7 @@ uint64_t MemRecursorCache::doDump(int fd)
   uint64_t count = 0;
 
   for (auto& mc : d_maps) {
-    auto map = mc.d_content.lock();
+    auto map = mc.lock();
     const auto& sidx = map->d_map.get<SequencedTag>();
 
     time_t now = time(nullptr);
diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 9afebd22fa..c9d0d66ba8 100644
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -224,7 +224,6 @@ private:
       }
     };
 
-    LockGuarded<LockedContent> d_content;
     std::atomic<uint64_t> d_entriesCount{0};
 
     LockGuardedTryHolder<LockedContent> lock()
@@ -237,6 +236,9 @@ private:
       ++locked->d_acquired_count;
       return locked;
     }
+
+  private:
+    LockGuarded<LockedContent> d_content;
   };
 
   vector<MapCombo> d_maps;
diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 1fe7c296e8..ea526bd1e4 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -60,7 +60,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
   DNSName lastLabel = qname.getLastLabel();
 
   auto& map = getMap(lastLabel);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
 
   negcache_t::const_iterator ni = content->d_map.find(tie(lastLabel, qtnull));
 
@@ -89,7 +89,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
 bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch)
 {
   auto& map = getMap(qname);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
 
   const auto& idx = content->d_map.get<NegCacheEntry>();
   auto range = idx.equal_range(qname);
@@ -124,7 +124,7 @@ void NegCache::add(const NegCacheEntry& ne)
 {
   bool inserted = false;
   auto& map = getMap(ne.d_name);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
   inserted = lruReplacingInsert<SequenceTag>(content->d_map, ne);
   if (inserted) {
     ++map.d_entriesCount;
@@ -141,7 +141,7 @@ void NegCache::add(const NegCacheEntry& ne)
 void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<time_t> capTTD)
 {
   auto& mc = getMap(qname);
-  auto map = mc.d_content.lock();
+  auto map = mc.lock();
   auto range = map->d_map.equal_range(tie(qname, qtype));
 
   if (range.first != range.second) {
@@ -160,7 +160,7 @@ void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype,
 size_t NegCache::count(const DNSName& qname)
 {
   auto& map = getMap(qname);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
   return content->d_map.count(tie(qname));
 }
 
@@ -173,7 +173,7 @@ size_t NegCache::count(const DNSName& qname)
 size_t NegCache::count(const DNSName& qname, const QType qtype)
 {
   auto& map = getMap(qname);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
   return content->d_map.count(tie(qname, qtype));
 }
 
@@ -189,7 +189,7 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
   size_t ret = 0;
   if (subtree) {
     for (auto& map : d_maps) {
-      auto m = map.d_content.lock();
+      auto m = map.lock();
       for (auto i = m->d_map.lower_bound(tie(name)); i != m->d_map.end();) {
         if (!i->d_name.isPartOf(name))
           break;
@@ -202,7 +202,7 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
   }
 
   auto& map = getMap(name);
-  auto content = map.d_content.lock();
+  auto content = map.lock();
   auto range = content->d_map.equal_range(tie(name));
   auto i = range.first;
   while (i != range.second) {
@@ -219,7 +219,7 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
 void NegCache::clear()
 {
   for (auto& map : d_maps) {
-    auto m = map.d_content.lock();
+    auto m = map.lock();
     m->d_map.clear();
     map.d_entriesCount = 0;
   }
@@ -246,7 +246,7 @@ size_t NegCache::dumpToFile(FILE* fp, const struct timeval& now)
   size_t ret = 0;
 
   for (auto& mc : d_maps) {
-    auto m = mc.d_content.lock();
+    auto m = mc.lock();
     auto& sidx = m->d_map.get<SequenceTag>();
     for (const NegCacheEntry& ne : sidx) {
       ret++;
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index a31f86287a..4abe4ed18b 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -113,7 +113,6 @@ private:
       uint64_t d_acquired_count{0};
       void invalidate() {}
     };
-    LockGuarded<LockedContent> d_content;
     std::atomic<uint64_t> d_entriesCount{0};
 
     LockGuardedTryHolder<MapCombo::LockedContent> lock()
@@ -126,6 +125,10 @@ private:
       ++locked->d_acquired_count;
       return locked;
     }
+
+  private:
+
+    LockGuarded<LockedContent> d_content;
   };
 
   vector<MapCombo> d_maps;