Add contention/acquired stats to sharded packet cache

diff --git a/pdns/recursordist/RECURSOR-MIB.txt b/pdns/recursordist/RECURSOR-MIB.txt
index 816fc47d0eeaa74fb29db154aba873953bb06f33..5d04fa74f61144673a3a54ec3fd60a164a3065a0 100644 (file)
--- a/pdns/recursordist/RECURSOR-MIB.txt
+++ b/pdns/recursordist/RECURSOR-MIB.txt
@@ -54,6 +54,9 @@ rec MODULE-IDENTITY
     REVISION "202209120000Z"
     DESCRIPTION "Added metrics for answers from auths by rcode"
 
+    REVISION "202302240000Z"
+    DESCRIPTION "Added metrics for sharded packet cache contrntion"
+
     ::= { powerdns 2 }
 
 powerdns               OBJECT IDENTIFIER ::= { enterprises 43315 }
@@ -1212,6 +1215,22 @@ authrcode15Count OBJECT-TYPE
         "Number of rcode 15 answers received"
     ::= { stats 144 }
 
+packetCacheContended OBJECT-TYPE
+    SYNTAX Counter64
+    MAX-ACCESS read-only
+    STATUS current
+    DESCRIPTION
+        "Number of contended packet cache lock acquisitions"
+    ::= { stats 145 }
+
+packetCacheAcquired OBJECT-TYPE
+    SYNTAX Counter64
+    MAX-ACCESS read-only
+    STATUS current
+    DESCRIPTION
+        "Number of packet cache lock acquisitions"
+    ::= { stats 146 }
+
 
 ---
 --- Traps / Notifications

diff --git a/pdns/recursordist/rec-snmp.cc b/pdns/recursordist/rec-snmp.cc
index 7ffce2acb184e15e209f853312f9181f1c5ae030..d7a4ad64f97c5c8b1c444e6cc97f05176a9b826f 100644 (file)
--- a/pdns/recursordist/rec-snmp.cc
+++ b/pdns/recursordist/rec-snmp.cc
@@ -167,6 +167,9 @@ static const oid rcode13AnswersOID[] = {RECURSOR_STATS_OID, 142};
 static const oid rcode14AnswersOID[] = {RECURSOR_STATS_OID, 143};
 static const oid rcode15AnswersOID[] = {RECURSOR_STATS_OID, 144};
 
+static const oid packetCacheContendedOID[] = {RECURSOR_STATS_OID, 145};
+static const oid packetCacheAcquiredOID[] = {RECURSOR_STATS_OID, 146};
+
 static std::unordered_map<oid, std::string> s_statsMap;
 
 /* We are never called for a GETNEXT if it's registered as a
@@ -401,6 +404,8 @@ RecursorSNMPAgent::RecursorSNMPAgent(const std::string& name, const std::string&
   registerCounter64Stat("non-resolving-nameserver-entries", nonResolvingNameserverEntriesOID, OID_LENGTH(nonResolvingNameserverEntriesOID));
   registerCounter64Stat("maintenance-usec", maintenanceUSecOID, OID_LENGTH(maintenanceUSecOID));
   registerCounter64Stat("maintenance-calls", maintenanceCallsOID, OID_LENGTH(maintenanceCallsOID));
+  registerCounter64Stat("packetcache-contended", packetCacheContendedOID, OID_LENGTH(packetCacheContendedOID));
+  registerCounter64Stat("packetcache-acquired", packetCacheAcquiredOID, OID_LENGTH(packetCacheAcquiredOID));
 
 #define RCODE(num) registerCounter64Stat("auth-" + RCode::to_short_s(num) + "-answers", rcode##num##AnswersOID, OID_LENGTH(rcode##num##AnswersOID))
   RCODE(0);

diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc
index 9bbc49df6e099baf523fdf3dc5657bac006e61ff..4e82cf6041414312d74376db6fdb0abc780b28f4 100644 (file)
--- a/pdns/recursordist/rec_channel_rec.cc
+++ b/pdns/recursordist/rec_channel_rec.cc
@@ -401,18 +401,19 @@ static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)(
 }
 
 // Does not follow the generic dump to file pattern, has a more complex lambda
-static RecursorControlChannel::Answer doDumpCache(int s)
+static RecursorControlChannel::Answer doDumpCache(int socket)
 {
-  auto fdw = getfd(s);
+  auto fdw = getfd(socket);
 
   if (fdw < 0) {
     return {1, "Error opening dump file for writing: " + stringerror() + "\n"};
   }
   uint64_t total = 0;
   try {
-    int fd = fdw;
-    total = g_recCache->doDump(fd, g_maxCacheEntries.load()) + g_negCache->doDump(fd, g_maxCacheEntries.load() / 8) +
-      (g_packetCache ? g_packetCache->doDump(fd) : 0) + dumpAggressiveNSECCache(fd);
+    total += g_recCache->doDump(fdw, g_maxCacheEntries.load());
+    total += g_negCache->doDump(fdw, g_maxCacheEntries.load() / 8);
+    total += g_packetCache ? g_packetCache->doDump(fdw) : 0;
+    total += dumpAggressiveNSECCache(fdw);
   }
   catch (...) {
   }
@@ -1265,7 +1266,9 @@ static void registerAllStats1()
   addGetStat("packetcache-hits", [] { return g_packetCache ? g_packetCache->getHits() : 0; });
   addGetStat("packetcache-misses", [] { return g_packetCache ? g_packetCache->getMisses() : 0; });
   addGetStat("packetcache-entries", [] { return g_packetCache ? g_packetCache->size() : 0; });
-  addGetStat("packetcache-bytes", [] { return g_packetCache ? g_packetCache->bytes() : 0; });;
+  addGetStat("packetcache-bytes", [] { return g_packetCache ? g_packetCache->bytes() : 0; });
+  addGetStat("packetcache-contended", []() { return g_packetCache ? g_packetCache->stats().first : 0; });
+  addGetStat("packetcache-acquired", []() { return g_packetCache ? g_packetCache->stats().second : 0; });
 
   addGetStat("aggressive-nsec-cache-entries", []() { return g_aggressiveNSECCache ? g_aggressiveNSECCache->getEntriesCount() : 0; });
   addGetStat("aggressive-nsec-cache-nsec-hits", []() { return g_aggressiveNSECCache ? g_aggressiveNSECCache->getNSECHits() : 0; });

diff --git a/pdns/recursordist/recpacketcache.cc b/pdns/recursordist/recpacketcache.cc
index 10fa9cbd1d9ea8adfae8ed4002ce1a0d13ea84ff..48fb138fc3f15e0494dc013b3d942cfb16ac83fb 100644 (file)
--- a/pdns/recursordist/recpacketcache.cc
+++ b/pdns/recursordist/recpacketcache.cc
@@ -53,6 +53,18 @@ uint64_t RecursorPacketCache::getMisses()
   return sum;
 }
 
+pair<uint64_t, uint64_t> RecursorPacketCache::stats()
+{
+  uint64_t contended = 0;
+  uint64_t acquired = 0;
+  for (auto& shard : d_maps) {
+    auto content = shard.lock();
+    contended += content->d_contended_count;
+    acquired += content->d_acquired_count;
+  }
+  return {contended, acquired};
+}
+
 uint64_t RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype, bool subtree)
 {
   uint64_t count = 0;
@@ -260,7 +272,7 @@ uint64_t RecursorPacketCache::doDump(int file)
     min = std::min(min, shardSize);
     max = std::max(max, shardSize);
     shardNum++;
-  for (const auto& entry : sidx) {
+    for (const auto& entry : sidx) {
       count++;
       try {
         fprintf(filePtr.get(), "%s %" PRId64 " %s  ; tag %d %s\n", entry.d_name.toString().c_str(), static_cast<int64_t>(entry.d_ttd - now), DNSRecordContent::NumberToType(entry.d_type).c_str(), entry.d_tag, entry.d_tcp ? "tcp" : "udp");

diff --git a/pdns/recursordist/recpacketcache.hh b/pdns/recursordist/recpacketcache.hh
index 8ebc46ecd4cd2913996b768f01c30bb786c54922..10a005b8975c35bc66237f4d1767ea666a0bd2bc 100644 (file)
--- a/pdns/recursordist/recpacketcache.hh
+++ b/pdns/recursordist/recpacketcache.hh
@@ -99,6 +99,7 @@ public:
   [[nodiscard]] uint64_t bytes();
   [[nodiscard]] uint64_t getHits();
   [[nodiscard]] uint64_t getMisses();
+  [[nodiscard]] pair<uint64_t, uint64_t> stats();
 
 private:
   struct Entry
@@ -207,7 +208,8 @@ private:
   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(MapCombo::LockedContent& shard, 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);
+  bool checkResponseMatches(MapCombo::LockedContent& shard, 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);
+
 public:
   void preRemoval(MapCombo::LockedContent& map, const Entry& entry)
   {

diff --git a/pdns/recursordist/ws-recursor.cc b/pdns/recursordist/ws-recursor.cc
index b1d11ae170df695e7de1b20a5b8c227cc7665a38..ae0cde21e1d5a7f5e74f10f00c629ec6bdb6af39 100644 (file)
--- a/pdns/recursordist/ws-recursor.cc
+++ b/pdns/recursordist/ws-recursor.cc
@@ -1070,7 +1070,15 @@ const std::map<std::string, MetricDefinition> MetricDefinitionStorage::d_metrics
 
   {"record-cache-contended",
    MetricDefinition(PrometheusMetricType::counter,
-                    "Number of contented record cache lock acquisitions")},
+                    "Number of contended record cache lock acquisitions")},
+
+  {"packetcache-acquired",
+   MetricDefinition(PrometheusMetricType::counter,
+                    "Number of packet cache lock acquisitions")},
+
+  {"packetcache-contended",
+   MetricDefinition(PrometheusMetricType::counter,
+                    "Number of contended packet cache lock acquisitions")},
 
   {"taskqueue-expired",
    MetricDefinition(PrometheusMetricType::counter,