From 5a9bd2d848b5ad0f51949270e2920fcc15ac3b0f Mon Sep 17 00:00:00 2001 From: Chris Hofstaedtler Date: Mon, 29 Mar 2021 15:49:39 +0200 Subject: [PATCH] domaincache: preserve domains added while replace data collection was running --- pdns/Makefile.am | 1 + pdns/auth-domaincache.cc | 44 +++++++++++++-- pdns/auth-domaincache.hh | 5 ++ pdns/test-auth-domaincache_cc.cc | 95 ++++++++++++++++++++++++++++++++ pdns/ueberbackend.cc | 1 + 5 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 pdns/test-auth-domaincache_cc.cc diff --git a/pdns/Makefile.am b/pdns/Makefile.am index c97adcf5fa..d8f6dd3ac4 100644 --- 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 c23ce7a1ee..8c5959dffd 100644 --- a/pdns/auth-domaincache.cc +++ b/pdns/auth-domaincache.cc @@ -106,10 +106,27 @@ void AuthDomainCache::replace(const vector>& 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& 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 162d98f511..dff7f4aab0 100644 --- a/pdns/auth-domaincache.hh +++ b/pdns/auth-domaincache.hh @@ -33,6 +33,7 @@ public: void replace(const vector>& 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> 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 index 0000000000..b19ccd88ed --- /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 + +#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> 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> 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> 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 6346f6f730..6aa13fe695 100644 --- a/pdns/ueberbackend.cc +++ b/pdns/ueberbackend.cc @@ -286,6 +286,7 @@ void UeberBackend::updateDomainCache() { } vector> domain_indices; + g_domainCache.setReplacePending(); for (vector::iterator i = backends.begin(); i != backends.end(); ++i ) { -- 2.47.2