auth: Enforce a strict maximum size for the packet and records caches

Before this change, both the query and packet caches in the authoritative
server can exceed their maximum size by a lot, until the next cleaning
cycle.
This is particularly nasty since the current cleaning algorithm will
never remove entries from the cache until they expire, as opposed to
what we do in the recursor, for example, where we nuke the least-recently
used entries, even if they are still valid, when the cache is full.
This commit changes that by removing the least recently inserted or
updated entry from the cache after inserting a new one when the cache
is full, thus enforcing the maximum size more strictly.

Note that this is really the least recently inserted/updated and not
the least recently used one, as is done in the recursor. Having a
proper LRU in the auth would require acquering a write lock for a
simple lookup, instead of a potentially concurrent read-lock at the
moment. We might want to consider changing that at some point, as
a LRU might be fairer and the lock contention might be very small
since the caches are sharded.

(cherry picked from commit d2814f81c77709d628a3fc036aff96ccc8a74c64)

diff --git a/pdns/auth-packetcache.cc b/pdns/auth-packetcache.cc
index b67c16d28a7b7a37d4feb992dbcece0fc114c724..45ac40991579b47a1f3637f513f98f448b8ac413 100644 (file)
--- a/pdns/auth-packetcache.cc
+++ b/pdns/auth-packetcache.cc
@@ -159,6 +159,7 @@ void AuthPacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxTTL)
         continue;
       }
 
+      moveCacheItemToBack<SequencedTag>(mc.d_map, iter);
       iter->value = entry.value;
       iter->ttd = now + ourttl;
       iter->created = now;
@@ -167,7 +168,15 @@ void AuthPacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxTTL)
 
     /* no existing entry found to refresh */
     mc.d_map.insert(entry);
-    (*d_statnumentries)++;
+
+    if (*d_statnumentries >= d_maxEntries) {
+      /* remove the least recently inserted or replaced entry */
+      auto& sidx = mc.d_map.get<SequencedTag>();
+      sidx.pop_front();
+    }
+    else {
+      (*d_statnumentries)++;
+    }
   }
 }
 

diff --git a/pdns/auth-packetcache.hh b/pdns/auth-packetcache.hh
index 2ba54daaa7d55c3c1c47643714ab4bc66281e5b1..7826ad64b0e4d27f4e32f8486525787fd9039128 100644 (file)
--- a/pdns/auth-packetcache.hh
+++ b/pdns/auth-packetcache.hh
@@ -98,6 +98,8 @@ private:
     indexed_by <
       hashed_non_unique<tag<HashTag>, member<CacheEntry,uint32_t,&CacheEntry::hash> >,
       ordered_non_unique<tag<NameTag>, member<CacheEntry,DNSName,&CacheEntry::qname>, CanonDNSNameCompare >,
+      /* Note that this sequence holds 'least recently inserted or replaced', not least recently used.
+         Making it a LRU would require taking a write-lock when fetching from the cache, making the RW-lock inefficient compared to a mutex */
       sequenced<tag<SequencedTag>>
       >
     > cmap_t;

diff --git a/pdns/auth-querycache.cc b/pdns/auth-querycache.cc
index 2e39d2d225f8a7421e13fc54a6819d37a9f41da5..bf5b57a3d55a3b9ecc1a5f4ca0e0358c0600ddd6 100644 (file)
--- a/pdns/auth-querycache.cc
+++ b/pdns/auth-querycache.cc
@@ -114,9 +114,17 @@ void AuthQueryCache::insert(const DNSName &qname, const QType& qtype, const vect
 
     if (!inserted) {
       mc.d_map.replace(place, val);
+      moveCacheItemToBack<SequencedTag>(mc.d_map, place);
     }
     else {
-      (*d_statnumentries)++;
+      if (*d_statnumentries >= d_maxEntries) {
+        /* remove the least recently inserted or replaced entry */
+        auto& sidx = mc.d_map.get<SequencedTag>();
+        sidx.pop_front();
+      }
+      else {
+        (*d_statnumentries)++;
+      }
     }
   }
 }

diff --git a/pdns/auth-querycache.hh b/pdns/auth-querycache.hh
index 90144a3c8527b394599daa40f4084645b28f1d77..e8f3a9f47c41b6d4fb592cb1c6df61d10924bc9a 100644 (file)
--- a/pdns/auth-querycache.hh
+++ b/pdns/auth-querycache.hh
@@ -80,6 +80,8 @@ private:
                                                          member<CacheEntry,uint16_t,&CacheEntry::qtype>,
                                                          member<CacheEntry,int, &CacheEntry::zoneID> > > ,
       ordered_non_unique<tag<NameTag>, member<CacheEntry,DNSName,&CacheEntry::qname>, CanonDNSNameCompare >,
+      /* Note that this sequence holds 'least recently inserted or replaced', not least recently used.
+         Making it a LRU would require taking a write-lock when fetching from the cache, making the RW-lock inefficient compared to a mutex */
       sequenced<tag<SequencedTag>>
                            >
   > cmap_t;