domaincache: preserve domains added while replace data collection was running

diff --git a/pdns/Makefile.am b/pdns/Makefile.am
index c97adcf5fa298309d86f3ec092a96b9453060cb5..d8f6dd3ac439162dc4d44ec9fcfeef5e06839d7c 100644 (file)
--- a/pdns/Makefile.am
+++ b/pdns/Makefile.am
@@ -1353,6 +1353,7 @@ testrunner_SOURCES = \
        stubresolver.hh stubresolver.cc \
        svc-records.cc svc-records.hh \
        test-arguments_cc.cc \
+       test-auth-domaincache_cc.cc \
        test-base32_cc.cc \
        test-base64_cc.cc \
        test-bindparser_cc.cc \

diff --git a/pdns/auth-domaincache.cc b/pdns/auth-domaincache.cc
index c23ce7a1eed17cd64aa102216ef937787c6ccadb..8c5959dffdd45e58974d0b9eb9a681a36adf544c 100644 (file)
--- a/pdns/auth-domaincache.cc
+++ b/pdns/auth-domaincache.cc
@@ -106,10 +106,27 @@ void AuthDomainCache::replace(const vector<tuple<DNSName, int>>& domain_indices)
     mc.d_map.emplace(domain, val);
   }
 
-  for (size_t mapIndex = 0; mapIndex < d_maps.size(); mapIndex++) {
-    auto& mc = d_maps[mapIndex];
-    WriteLock l(mc.d_mut);
-    mc.d_map = std::move(newMaps[mapIndex].d_map);
+  {
+    WriteLock globalLock(d_mut);
+    if (d_replacePending) {
+      // add/replace all domains created while data collection for replace() was already running.
+      for (const tuple<DNSName, int>& tup : d_pendingAdds) {
+        const DNSName& domain = tup.get<0>();
+        CacheValue val;
+        val.zoneId = tup.get<1>();
+        auto& mc = newMaps[getMapIndex(domain)];
+        mc.d_map[domain] = val;
+      }
+    }
+
+    for (size_t mapIndex = 0; mapIndex < d_maps.size(); mapIndex++) {
+      auto& mc = d_maps[mapIndex];
+      WriteLock mcLock(mc.d_mut);
+      mc.d_map = std::move(newMaps[mapIndex].d_map);
+    }
+
+    d_pendingAdds.clear();
+    d_replacePending = false;
   }
 
   d_statnumentries->store(count);
@@ -120,13 +137,30 @@ void AuthDomainCache::add(const DNSName& domain, const int zoneId)
   if (!d_ttl)
     return;
 
+  {
+    WriteLock globalLock(d_mut);
+    if (d_replacePending) {
+      d_pendingAdds.push_back({domain, zoneId});
+    }
+  }
+
   CacheValue val;
   val.zoneId = zoneId;
 
   int mapIndex = getMapIndex(domain);
   {
     auto& mc = d_maps[mapIndex];
-    WriteLock l(mc.d_mut);
+    WriteLock mcLock(mc.d_mut);
     mc.d_map.emplace(domain, val);
   }
 }
+
+void AuthDomainCache::setReplacePending()
+{
+  if (!d_ttl)
+    return;
+
+  WriteLock globalLock(d_mut);
+  d_replacePending = true;
+  d_pendingAdds.clear();
+}

diff --git a/pdns/auth-domaincache.hh b/pdns/auth-domaincache.hh
index 162d98f51107d6daa3a41abbc1d333d52e5f946a..dff7f4aab0c84eba9dcbe0ae8f1272f817a73c35 100644 (file)
--- a/pdns/auth-domaincache.hh
+++ b/pdns/auth-domaincache.hh
@@ -33,6 +33,7 @@ public:
 
   void replace(const vector<tuple<DNSName, int>>& domains);
   void add(const DNSName& domain, const int zoneId);
+  void setReplacePending();  //!< call this when data collection for subsequent replace() call starts.
 
   bool getEntry(const DNSName& domain, int& zoneId);
 
@@ -86,6 +87,10 @@ private:
   AtomicCounter* d_statnumentries;
 
   time_t d_ttl{0};
+
+  ReadWriteLock d_mut;
+  std::vector<tuple<DNSName, int>> d_pendingAdds;
+  bool d_replacePending{false};
 };
 
 extern AuthDomainCache g_domainCache;

diff --git a/pdns/test-auth-domaincache_cc.cc b/pdns/test-auth-domaincache_cc.cc
new file mode 100644 (file)
index 0000000..b19ccd8
--- /dev/null
+++ b/pdns/test-auth-domaincache_cc.cc
@@ -0,0 +1,95 @@
+
+/*
+    PowerDNS Versatile Database Driven Nameserver
+    Copyright (C) 2021  PowerDNS.COM BV
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License version 2
+    as published by the Free Software Foundation
+
+    Additionally, the license of this program contains a special
+    exception which allows to distribute the program in binary form when
+    it is linked against OpenSSL.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+*/
+
+#define BOOST_TEST_DYN_LINK
+#define BOOST_TEST_NO_MAIN
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <boost/test/unit_test.hpp>
+
+#include "auth-domaincache.hh"
+#include "misc.hh"
+
+BOOST_AUTO_TEST_SUITE(test_auth_domaincache_cc)
+
+BOOST_AUTO_TEST_CASE(test_replace) {
+  AuthDomainCache cache;
+  cache.setTTL(3600);
+
+  vector<tuple<DNSName, int>> domain_indices{
+    {DNSName("example.org."), 1},
+  };
+  cache.replace(domain_indices);
+
+  int zoneId = 0;
+  bool found = cache.getEntry(DNSName("example.org."), zoneId);
+  if (!found || zoneId == 0) {
+    BOOST_FAIL("domain added in replace() not found");
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_add_while_pending_replace) {
+  AuthDomainCache cache;
+  cache.setTTL(3600);
+
+  vector<tuple<DNSName, int>> domain_indices{
+    {DNSName("powerdns.org."), 1}
+  };
+  cache.setReplacePending();
+  cache.add(DNSName("example.org."), 2);
+  cache.replace(domain_indices);
+
+  int zoneId = 0;
+  bool found = cache.getEntry(DNSName("example.org."), zoneId);
+  if (!found || zoneId == 0) {
+    BOOST_FAIL("domain added while replace was pending not found");
+  }
+}
+
+// Add domain using .add(), but also in the .replace() data
+BOOST_AUTO_TEST_CASE(test_add_while_pending_replace_duplicate) {
+  AuthDomainCache cache;
+  cache.setTTL(3600);
+
+  vector<tuple<DNSName, int>> domain_indices{
+    {DNSName("powerdns.org."), 1},
+    {DNSName("example.org."), 2},
+  };
+  cache.setReplacePending();
+  cache.add(DNSName("example.org."), 3);
+  cache.replace(domain_indices);
+
+  int zoneId = 0;
+  bool found = cache.getEntry(DNSName("example.org."), zoneId);
+  if (!found || zoneId == 0) {
+    BOOST_FAIL("domain added while replace was pending not found");
+  }
+  if (zoneId != 3) {
+    BOOST_FAIL(string("zoneId got overwritten using replace() data (zoneId=") + std::to_string(zoneId) + ")");
+  }
+}
+
+BOOST_AUTO_TEST_SUITE_END();

diff --git a/pdns/ueberbackend.cc b/pdns/ueberbackend.cc
index 6346f6f73001c8e1d7204745b3c4a494279a5286..6aa13fe69569628904bb47da66fb3e9679e8cf4f 100644 (file)
--- a/pdns/ueberbackend.cc
+++ b/pdns/ueberbackend.cc
@@ -286,6 +286,7 @@ void UeberBackend::updateDomainCache() {
   }
 
   vector<tuple<DNSName, int>> domain_indices;
+  g_domainCache.setReplacePending();
 
   for (vector<DNSBackend*>::iterator i = backends.begin(); i != backends.end(); ++i )
   {