rec: Only update the ECS cache index when needed

We don't need to update the ECS index when replacing an existing
entry, except if the entry has expired, because then we might have
removed it from the ECS index.

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index ea9a4612f542f701045344f807f4312452a9f1a3..23dee223fb38cecceaaf292a54b5518cb097c092 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -272,14 +272,21 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt
     isNew = true;
   }
 
-  /* don't bother building an ecsIndex if we don't have any netmask-specific entries */
-  if (ednsmask && !ednsmask->empty()) {
-    auto ecsIndexKey = boost::make_tuple(qname, qt.getCode());
-    auto ecsIndex = d_ecsIndex.find(ecsIndexKey);
-    if (ecsIndex == d_ecsIndex.end()) {
-      ecsIndex = d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
+  /* if we are inserting a new entry or updating an expired one (in which case the
+     ECS index might have been removed but the entry still exists because it has not
+     been garbage collected yet) we might need to update the ECS index.
+     Otherwise it should already be indexed and we don't need to update it.
+  */
+  if (isNew || stored->d_ttd <= now) {
+    /* don't bother building an ecsIndex if we don't have any netmask-specific entries */
+    if (ednsmask && !ednsmask->empty()) {
+      auto ecsIndexKey = boost::make_tuple(qname, qt.getCode());
+      auto ecsIndex = d_ecsIndex.find(ecsIndexKey);
+      if (ecsIndex == d_ecsIndex.end()) {
+        ecsIndex = d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first;
+      }
+      ecsIndex->addMask(*ednsmask);
     }
-    ecsIndex->addMask(*ednsmask);
   }
 
   time_t maxTTD=std::numeric_limits<time_t>::max();

diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 7d42aa03faf1b68f44fb042395ba50c7a918b6cc..0780b62b2fcd4c38458e99c138a7ea01a4f0a877 100644 (file)
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -679,6 +679,16 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheECSIndex) {
   BOOST_CHECK_EQUAL(MRC.ecsIndexSize(), 0);
   BOOST_CHECK_EQUAL(MRC.size(), 1);
 
+  /* add back the entry while it still exists in the cache but has been removed from the ECS index.
+     It should be added back to the ECS index, and we should be able to retrieve it */
+  MRC.replace(now + ttl + 1, power, QType(QType::A), records, signatures, authRecords, true, Netmask("192.0.2.0/31"));
+  BOOST_CHECK_EQUAL(MRC.size(), 1);
+  BOOST_CHECK_EQUAL(MRC.ecsIndexSize(), 1);
+  retrieved.clear();
+  BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("192.0.2.1")), ttd - now);
+  BOOST_REQUIRE_EQUAL(retrieved.size(), 1);
+  BOOST_CHECK_EQUAL(getRR<ARecordContent>(retrieved.at(0))->getCA().toString(), dr1Content.toString());
+
   /* wipe everything */
   MRC.doPrune(0);
   BOOST_CHECK_EQUAL(MRC.size(), 0);