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.

diff --git a/pdns/auth-packetcache.cc b/pdns/auth-packetcache.cc
index 48b937acf31338535ac8c2b7f36e6a26ea99104c..094c1f63ba1c15aae7fdca130378d57c797d3ed7 100644 (file)
--- a/pdns/auth-packetcache.cc
+++ b/pdns/auth-packetcache.cc
@@ -154,6 +154,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;
@@ -162,7 +163,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 e0191dc95008bede5c077d6d5a70a6110a4a8563..91b41528bcc1c09562dcb445093a2f560df429b1 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 2dc8c3254b1c950feabfdedce286aa5dcfe90857..e790e9e16523ec67c9ee30efa9160164ab8167bf 100644 (file)
--- a/pdns/auth-querycache.cc
+++ b/pdns/auth-querycache.cc
@@ -109,9 +109,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 193f91bf92b8d37080d755d1557be46c5c6c41c0..2dd27c835d9e4bae4e87e39f3a27d42e8f82aedc 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;