From: Remi Gacogne Date: Thu, 20 May 2021 06:23:31 +0000 (+0200) Subject: rec: Move the record caches to LockGuarded (WIP: size() should not need a lock) X-Git-Tag: dnsdist-1.7.0-alpha1~62^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41fcded62f108468c881e48f54c1b0297686ab4e;p=thirdparty%2Fpdns.git rec: Move the record caches to LockGuarded (WIP: size() should not need a lock) --- diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh index 4962ff6357..fecdbc1ab2 100644 --- a/pdns/cachecleaner.hh +++ b/pdns/cachecleaner.hh @@ -21,7 +21,6 @@ */ #pragma once -#include #include #include "dnsname.hh" @@ -50,7 +49,6 @@ template void pruneCollection(C& container, for (auto iter = sidx.begin(); iter != sidx.end() && tried < lookAt ; ++tried) { if (iter->getTTD() < now) { - container.preRemoval(*iter); iter = sidx.erase(iter); erased++; } @@ -72,7 +70,6 @@ template void pruneCollection(C& container, // just lob it off from the beginning auto iter = sidx.begin(); for (size_t i = 0; i < toTrim && iter != sidx.end(); i++) { - container.preRemoval(*iter); iter = sidx.erase(iter); } } @@ -145,17 +142,17 @@ template uint64_t pruneMutexCollectionsVect if (maps_size == 0) return 0; - for (auto& mc : maps) { - const typename C::lock l(mc); - mc.invalidate(); - auto& sidx = boost::multi_index::get(mc.d_map); + for (auto& lockGuardedMap : maps) { + auto mc = C::lock(lockGuardedMap); + mc->invalidate(); + auto& sidx = boost::multi_index::get(mc->d_map); uint64_t erased = 0, lookedAt = 0; for (auto i = sidx.begin(); i != sidx.end(); lookedAt++) { if (i->getTTD() < now) { - container.preRemoval(*i); + container.preRemoval(*mc, *i); i = sidx.erase(i); erased++; - mc.d_entriesCount--; + mc->d_entriesCount--; } else { ++i; } @@ -179,15 +176,15 @@ template uint64_t pruneMutexCollectionsVect while (true) { size_t pershard = toTrim / maps_size + 1; - for (auto& mc : maps) { - const typename C::lock l(mc); - mc.invalidate(); - auto& sidx = boost::multi_index::get(mc.d_map); + for (auto& lockGuardedMap : maps) { + auto mc = C::lock(lockGuardedMap); + mc->invalidate(); + auto& sidx = boost::multi_index::get(mc->d_map); size_t removed = 0; for (auto i = sidx.begin(); i != sidx.end() && removed < pershard; removed++) { - container.preRemoval(*i); + container.preRemoval(*mc, *i); i = sidx.erase(i); - mc.d_entriesCount--; + mc->d_entriesCount--; totErased++; toTrim--; if (toTrim == 0) { diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh index cf821a6d07..edae2152f7 100644 --- a/pdns/dnsseckeeper.hh +++ b/pdns/dnsseckeeper.hh @@ -304,14 +304,6 @@ private: static AtomicCounter s_ops; static time_t s_last_prune; static size_t s_maxEntries; - -public: - void preRemoval(const KeyCacheEntry&) - { - } - void preRemoval(const METACacheEntry&) - { - } }; class DNSPacket; diff --git a/pdns/lock.hh b/pdns/lock.hh index fbf94bf836..d49bb7a3f7 100644 --- a/pdns/lock.hh +++ b/pdns/lock.hh @@ -200,6 +200,11 @@ public: return d_lock.owns_lock(); } + void lock() + { + d_lock.lock(); + } + private: std::unique_lock d_lock; T& d_value; diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh index de6e3c719b..7ef8a99e9d 100644 --- a/pdns/recpacketcache.hh +++ b/pdns/recpacketcache.hh @@ -128,9 +128,4 @@ private: static bool qrMatch(const packetCache_t::index::type::iterator& iter, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass); bool checkResponseMatches(std::pair::type::iterator, packetCache_t::index::type::iterator> range, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, OptPBData* pbdata); - -public: - void preRemoval(const Entry& entry) - { - } }; diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 76e702fba1..972b6010a6 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -22,8 +22,9 @@ MemRecursorCache::MemRecursorCache(size_t mapsCount) : d_maps(mapsCount) size_t MemRecursorCache::size() { size_t count = 0; - for (auto& map : d_maps) { - count += map.d_entriesCount; + for (auto& lockGuardedMap : d_maps) { + auto map = lock(lockGuardedMap); + count += map->d_entriesCount; } return count; } @@ -31,10 +32,10 @@ size_t MemRecursorCache::size() pair MemRecursorCache::stats() { uint64_t c = 0, a = 0; - for (auto& map : d_maps) { - const lock l(map); - c += map.d_contended_count; - a += map.d_acquired_count; + for (auto& lockGuardedMap : d_maps) { + auto map = lock(lockGuardedMap); + c += map->d_contended_count; + a += map->d_acquired_count; } return pair(c, a); } @@ -44,8 +45,8 @@ size_t MemRecursorCache::ecsIndexSize() // XXX! size_t count = 0; for (auto& map : d_maps) { - const lock l(map); - count += map.d_ecsIndex.size(); + auto m = lock(map); + count += m->d_ecsIndex.size(); } return count; } @@ -55,8 +56,8 @@ size_t MemRecursorCache::bytes() { size_t ret = 0; for (auto& map : d_maps) { - const lock l(map); - for (const auto& i : map.d_map) { + auto m = lock(map); + for (const auto& i : m->d_map) { ret += sizeof(struct CacheEntry); ret += i.d_qname.toString().length(); for (const auto& record : i.d_records) { @@ -272,22 +273,22 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b *wasAuth = true; } - auto& map = getMap(qname); - const lock l(map); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); /* If we don't have any netmask-specific entries at all, let's just skip this to be able to use the nice d_cachecache hack. */ - if (qtype != QType::ANY && !map.d_ecsIndex.empty() && !routingTag) { + if (qtype != QType::ANY && !map->d_ecsIndex.empty() && !routingTag) { if (qtype == QType::ADDR) { time_t ret = -1; - auto entryA = getEntryUsingECSIndex(map, now, qname, QType::A, requireAuth, who); - if (entryA != map.d_map.end()) { - ret = handleHit(map, entryA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); + auto entryA = getEntryUsingECSIndex(*map, now, qname, QType::A, requireAuth, who); + if (entryA != map->d_map.end()) { + ret = handleHit(*map, entryA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); } - auto entryAAAA = getEntryUsingECSIndex(map, now, qname, QType::AAAA, requireAuth, who); - if (entryAAAA != map.d_map.end()) { - time_t ttdAAAA = handleHit(map, entryAAAA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); + auto entryAAAA = getEntryUsingECSIndex(*map, now, qname, QType::AAAA, requireAuth, who); + if (entryAAAA != map->d_map.end()) { + time_t ttdAAAA = handleHit(*map, entryAAAA, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); if (ret > 0) { ret = std::min(ret, ttdAAAA); } else { @@ -302,9 +303,9 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b return ret > 0 ? (ret - now) : ret; } else { - auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who); - if (entry != map.d_map.end()) { - time_t ret = handleHit(map, entry, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); + auto entry = getEntryUsingECSIndex(*map, now, qname, qtype, requireAuth, who); + if (entry != map->d_map.end()) { + time_t ret = handleHit(*map, entry, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); if (state && cachedState) { *state = *cachedState; } @@ -315,17 +316,17 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b } if (routingTag) { - auto entries = getEntries(map, qname, qt, routingTag); + auto entries = getEntries(*map, qname, qt, routingTag); bool found = false; time_t ttd; if (entries.first != entries.second) { OrderedTagIterator_t firstIndexIterator; for (auto i=entries.first; i != entries.second; ++i) { - firstIndexIterator = map.d_map.project(i); + firstIndexIterator = map->d_map.project(i); if (i->d_ttd <= now) { - moveCacheItemToFront(map.d_map, firstIndexIterator); + moveCacheItemToFront(map->d_map, firstIndexIterator); continue; } @@ -333,7 +334,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b continue; } found = true; - ttd = handleHit(map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); + ttd = handleHit(*map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); if (qt != QType::ANY && qt != QType::ADDR) { // normally if we have a hit, we are done break; @@ -350,7 +351,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b } } // Try (again) without tag - auto entries = getEntries(map, qname, qt, boost::none); + auto entries = getEntries(*map, qname, qt, boost::none); if (entries.first != entries.second) { OrderedTagIterator_t firstIndexIterator; @@ -358,10 +359,10 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b time_t ttd; for (auto i=entries.first; i != entries.second; ++i) { - firstIndexIterator = map.d_map.project(i); + firstIndexIterator = map->d_map.project(i); if (i->d_ttd <= now) { - moveCacheItemToFront(map.d_map, firstIndexIterator); + moveCacheItemToFront(map->d_map, firstIndexIterator); continue; } @@ -370,7 +371,7 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b } found = true; - ttd = handleHit(map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); + ttd = handleHit(*map, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone); if (qt != QType::ANY && qt != QType::ADDR) { // normally if we have a hit, we are done break; @@ -388,10 +389,10 @@ time_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType qt, b void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, const vector& content, const vector>& signatures, const std::vector>& authorityRecs, bool auth, const DNSName& authZone, boost::optional ednsmask, const OptTag& routingTag, vState state, boost::optional from) { - auto& map = getMap(qname); - const lock l(map); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); - map.d_cachecachevalid = false; + map->d_cachecachevalid = false; if (ednsmask) { ednsmask = ednsmask->getNormalized(); } @@ -400,10 +401,10 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, // We only store an ednsmask if we do not have a tag and we do have a mask. auto key = boost::make_tuple(qname, qt.getCode(), ednsmask ? routingTag : boost::none, (ednsmask && !routingTag) ? *ednsmask : Netmask()); bool isNew = false; - cache_t::iterator stored = map.d_map.find(key); - if (stored == map.d_map.end()) { - stored = map.d_map.insert(CacheEntry(key, auth)).first; - map.d_entriesCount++; + cache_t::iterator stored = map->d_map.find(key); + if (stored == map->d_map.end()) { + stored = map->d_map.insert(CacheEntry(key, auth)).first; + map->d_entriesCount++; isNew = true; } @@ -416,9 +417,9 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, /* don't bother building an ecsIndex if we don't have any netmask-specific entries */ if (!routingTag && ednsmask && !ednsmask->empty()) { auto ecsIndexKey = boost::make_tuple(qname, qt.getCode()); - auto ecsIndex = map.d_ecsIndex.find(ecsIndexKey); - if (ecsIndex == map.d_ecsIndex.end()) { - ecsIndex = map.d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first; + auto ecsIndex = map->d_ecsIndex.find(ecsIndexKey); + if (ecsIndex == map->d_ecsIndex.end()) { + ecsIndex = map->d_ecsIndex.insert(ECSIndexEntry(qname, qt.getCode())).first; } ecsIndex->addMask(*ednsmask); } @@ -480,10 +481,10 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType qt, } if (!isNew) { - moveCacheItemToBack(map.d_map, stored); + moveCacheItemToBack(map->d_map, stored); } ce.d_submitted = false; - map.d_map.replace(stored, ce); + map->d_map.replace(stored, ce); } size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType qtype) @@ -491,50 +492,50 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType size_t count = 0; if (!sub) { - auto& map = getMap(name); - const lock l(map); - map.d_cachecachevalid = false; - auto& idx = map.d_map.get(); + auto& lockGuardedMap = getMap(name); + auto map = lock(lockGuardedMap); + map->d_cachecachevalid = false; + auto& idx = map->d_map.get(); auto range = idx.equal_range(name); auto i = range.first; while (i != range.second) { if (i->d_qtype == qtype || qtype == 0xffff) { i = idx.erase(i); count++; - map.d_entriesCount--; + map->d_entriesCount--; } else { ++i; } } if (qtype == 0xffff) { - auto& ecsIdx = map.d_ecsIndex.get(); + auto& ecsIdx = map->d_ecsIndex.get(); auto ecsIndexRange = ecsIdx.equal_range(name); ecsIdx.erase(ecsIndexRange.first, ecsIndexRange.second); } else { - auto& ecsIdx = map.d_ecsIndex.get(); + auto& ecsIdx = map->d_ecsIndex.get(); auto ecsIndexRange = ecsIdx.equal_range(tie(name, qtype)); ecsIdx.erase(ecsIndexRange.first, ecsIndexRange.second); } } else { - for (auto& map : d_maps) { - const lock l(map); - map.d_cachecachevalid = false; - auto& idx = map.d_map.get(); + for (auto& lockGuardedMap : d_maps) { + auto map = lock(lockGuardedMap); + map->d_cachecachevalid = false; + auto& idx = map->d_map.get(); for (auto i = idx.lower_bound(name); i != idx.end(); ) { if (!i->d_qname.isPartOf(name)) break; if (i->d_qtype == qtype || qtype == 0xffff) { count++; i = idx.erase(i); - map.d_entriesCount--; + map->d_entriesCount--; } else { ++i; } } - auto& ecsIdx = map.d_ecsIndex.get(); + auto& ecsIdx = map->d_ecsIndex.get(); for (auto i = ecsIdx.lower_bound(name); i != ecsIdx.end(); ) { if (!i->d_qname.isPartOf(name)) break; @@ -552,10 +553,10 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType // Name should be doLimitTime or so bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, const QType qtype, uint32_t newTTL) { - auto& map = getMap(name); - const lock l(map); - cache_t::iterator iter = map.d_map.find(tie(name, qtype)); - if (iter == map.d_map.end()) { + auto& lockGuardedMap = getMap(name); + auto map = lock(lockGuardedMap); + cache_t::iterator iter = map->d_map.find(tie(name, qtype)); + if (iter == map->d_map.end()) { return false; } @@ -565,13 +566,13 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, const QType q uint32_t maxTTL = static_cast(ce.d_ttd - now); if (maxTTL > newTTL) { - map.d_cachecachevalid = false; + map->d_cachecachevalid = false; time_t newTTD = now + newTTL; if (ce.d_ttd > newTTD) { ce.d_ttd = newTTD; - map.d_map.replace(iter, ce); + map->d_map.replace(iter, ce); } return true; } @@ -588,13 +589,13 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, throw std::runtime_error("Trying to update the DNSSEC validation status of several (via ADDR) records for " + qname.toLogString()); } - auto& map = getMap(qname); - const lock l(map); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); bool updated = false; - if (!map.d_ecsIndex.empty() && !routingTag) { - auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who); - if (entry == map.d_map.end()) { + if (!map->d_ecsIndex.empty() && !routingTag) { + auto entry = getEntryUsingECSIndex(*map, now, qname, qtype, requireAuth, who); + if (entry == map->d_map.end()) { return false; } @@ -605,10 +606,10 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, return true; } - auto entries = getEntries(map, qname, qt, routingTag); + auto entries = getEntries(*map, qname, qt, routingTag); for(auto i = entries.first; i != entries.second; ++i) { - auto firstIndexIterator = map.d_map.project(i); + auto firstIndexIterator = map->d_map.project(i); if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) { continue; @@ -641,9 +642,9 @@ uint64_t MemRecursorCache::doDump(int fd) fprintf(fp.get(), "; main record cache dump follows\n;\n"); uint64_t count = 0; - for (auto& map : d_maps) { - const lock l(map); - const auto& sidx = map.d_map.get(); + for (auto& lockGuardedMap : d_maps) { + auto map = lock(lockGuardedMap); + const auto& sidx = map->d_map.get(); time_t now = time(nullptr); for (const auto& i : sidx) { diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 0fe249f170..dbe7435400 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -22,7 +22,6 @@ #pragma once #include #include -#include #include "dns.hh" #include "qtype.hh" #include "misc.hh" @@ -38,6 +37,7 @@ #include #include #include "iputils.hh" +#include "lock.hh" #include "validate.hh" #undef max @@ -212,7 +212,6 @@ private: DNSName d_cachedqname; OptTag d_cachedrtag; Entries d_cachecache; - std::mutex mutex; bool d_cachecachevalid{false}; std::atomic d_entriesCount{0}; uint64_t d_contended_count{0}; @@ -224,10 +223,10 @@ private: } }; - vector d_maps; - MapCombo& getMap(const DNSName &qname) + vector> d_maps; + LockGuarded& getMap(const DNSName &qname) { - return d_maps[qname.hash() % d_maps.size()]; + return d_maps.at(qname.hash() % d_maps.size()); } static time_t fakeTTD(OrderedTagIterator_t& entry, const DNSName& qname, QType qtype, time_t ret, time_t now, uint32_t origTTL, bool refresh); @@ -239,30 +238,24 @@ private: time_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, uint32_t& origTTL, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, boost::optional& state, bool* wasAuth, DNSName* authZone); public: - struct lock { - lock(MapCombo& map) : m(map.mutex) - { - if (!m.try_lock()) { - m.lock(); - map.d_contended_count++; - } - map.d_acquired_count++; - } - ~lock() { - m.unlock(); + static LockGuardedTryHolder lock(LockGuarded& map) + { + auto locked = map.try_lock(); + if (!locked.owns_lock()) { + locked.lock(); + locked->d_contended_count++; } - private: - std::mutex &m; - }; + ++locked->d_acquired_count; + return locked; + } - void preRemoval(const CacheEntry& entry) + void preRemoval(MapCombo& map, const CacheEntry& entry) { if (entry.d_netmask.empty()) { return; } auto key = tie(entry.d_qname, entry.d_qtype); - auto& map = getMap(entry.d_qname); auto ecsIndexEntry = map.d_ecsIndex.find(key); if (ecsIndexEntry != map.d_ecsIndex.end()) { ecsIndexEntry->removeNetmask(entry.d_netmask); diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc index 53dcb6f76a..717ce75b90 100644 --- a/pdns/recursordist/negcache.cc +++ b/pdns/recursordist/negcache.cc @@ -31,11 +31,12 @@ NegCache::NegCache(size_t mapsCount) : { } -size_t NegCache::size() const +size_t NegCache::size() { size_t count = 0; - for (const auto& map : d_maps) { - count += map.d_entriesCount; + for (auto& lockGuardedMap : d_maps) { + auto map = lock(lockGuardedMap); + count += map->d_entriesCount; } return count; } @@ -59,19 +60,19 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N static const QType qtnull(0); DNSName lastLabel = qname.getLastLabel(); - auto& map = getMap(lastLabel); - const lock l(map); + auto& lockGuardedMap = getMap(lastLabel); + auto map = lock(lockGuardedMap); - negcache_t::const_iterator ni = map.d_map.find(tie(lastLabel, qtnull)); + negcache_t::const_iterator ni = map->d_map.find(tie(lastLabel, qtnull)); - while (ni != map.d_map.end() && ni->d_name == lastLabel && ni->d_auth.isRoot() && ni->d_qtype == qtnull) { + while (ni != map->d_map.end() && ni->d_name == lastLabel && ni->d_auth.isRoot() && ni->d_qtype == qtnull) { // We have something if (now.tv_sec < ni->d_ttd) { ne = *ni; - moveCacheItemToBack(map.d_map, ni); + moveCacheItemToBack(map->d_map, ni); return true; } - moveCacheItemToFront(map.d_map, ni); + moveCacheItemToFront(map->d_map, ni); ++ni; } return false; @@ -88,10 +89,10 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N */ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch) { - auto& map = getMap(qname); - const lock l(map); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); - const auto& idx = map.d_map.get(); + const auto& idx = map->d_map.get(); auto range = idx.equal_range(qname); auto ni = range.first; @@ -99,16 +100,16 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva // We have an entry if ((!typeMustMatch && ni->d_qtype.getCode() == 0) || ni->d_qtype == qtype) { // We match the QType or the whole name is denied - auto firstIndexIterator = map.d_map.project(ni); + auto firstIndexIterator = map->d_map.project(ni); if (now.tv_sec < ni->d_ttd) { // Not expired ne = *ni; - moveCacheItemToBack(map.d_map, firstIndexIterator); + moveCacheItemToBack(map->d_map, firstIndexIterator); return true; } // expired - moveCacheItemToFront(map.d_map, firstIndexIterator); + moveCacheItemToFront(map->d_map, firstIndexIterator); } ++ni; } @@ -122,11 +123,11 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva */ void NegCache::add(const NegCacheEntry& ne) { - auto& map = getMap(ne.d_name); - const lock l(map); - bool inserted = lruReplacingInsert(map.d_map, ne); + auto& lockGuardedMap = getMap(ne.d_name); + auto map = lock(lockGuardedMap); + bool inserted = lruReplacingInsert(map->d_map, ne); if (inserted) { - map.d_entriesCount++; + ++map->d_entriesCount; } } @@ -139,9 +140,9 @@ void NegCache::add(const NegCacheEntry& ne) */ void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional capTTD) { - auto& map = getMap(qname); - const lock l(map); - auto range = map.d_map.equal_range(tie(qname, qtype)); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); + auto range = map->d_map.equal_range(tie(qname, qtype)); if (range.first != range.second) { range.first->d_validationState = newState; @@ -156,11 +157,11 @@ void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, * * \param qname The name of the entries to be counted */ -size_t NegCache::count(const DNSName& qname) const +size_t NegCache::count(const DNSName& qname) { - const auto& map = getMap(qname); - const lock l(map); - return map.d_map.count(tie(qname)); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); + return map->d_map.count(tie(qname)); } /*! @@ -169,11 +170,11 @@ size_t NegCache::count(const DNSName& qname) const * \param qname The name of the entries to be counted * \param qtype The type of the entries to be counted */ -size_t NegCache::count(const DNSName& qname, const QType qtype) const +size_t NegCache::count(const DNSName& qname, const QType qtype) { - const auto& map = getMap(qname); - const lock l(map); - return map.d_map.count(tie(qname, qtype)); + auto& lockGuardedMap = getMap(qname); + auto map = lock(lockGuardedMap); + return map->d_map.count(tie(qname, qtype)); } /*! @@ -187,27 +188,27 @@ size_t NegCache::wipe(const DNSName& name, bool subtree) { size_t ret = 0; if (subtree) { - for (auto& m : d_maps) { - const lock l(m); - for (auto i = m.d_map.lower_bound(tie(name)); i != m.d_map.end();) { + for (auto& map : d_maps) { + auto m = lock(map); + for (auto i = m->d_map.lower_bound(tie(name)); i != m->d_map.end();) { if (!i->d_name.isPartOf(name)) break; - i = m.d_map.erase(i); + i = m->d_map.erase(i); ret++; - m.d_entriesCount--; + --m->d_entriesCount; } } return ret; } - auto& map = getMap(name); - const lock l(map); - auto range = map.d_map.equal_range(tie(name)); + auto& lockGuardedMap = getMap(name); + auto map = lock(lockGuardedMap); + auto range = map->d_map.equal_range(tie(name)); auto i = range.first; while (i != range.second) { - i = map.d_map.erase(i); + i = map->d_map.erase(i); ret++; - map.d_entriesCount--; + --map->d_entriesCount; } return ret; } @@ -217,10 +218,10 @@ size_t NegCache::wipe(const DNSName& name, bool subtree) */ void NegCache::clear() { - for (auto& m : d_maps) { - const lock l(m); - m.d_map.clear(); - m.d_entriesCount = 0; + for (auto& map : d_maps) { + auto m = lock(map); + m->d_map.clear(); + m->d_entriesCount = 0; } } @@ -240,13 +241,13 @@ void NegCache::prune(size_t maxEntries) * * \param fp A pointer to an open FILE object */ -size_t NegCache::dumpToFile(FILE* fp, const struct timeval& now) const +size_t NegCache::dumpToFile(FILE* fp, const struct timeval& now) { size_t ret = 0; - for (const auto& m : d_maps) { - const lock l(m); - auto& sidx = m.d_map.get(); + for (auto& map : d_maps) { + auto m = lock(map); + auto& sidx = m->d_map.get(); for (const NegCacheEntry& ne : sidx) { ret++; int64_t ttl = ne.d_ttd - now.tv_sec; diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh index 7e07b9e08d..1a875e5960 100644 --- a/pdns/recursordist/negcache.hh +++ b/pdns/recursordist/negcache.hh @@ -21,7 +21,6 @@ */ #pragma once -#include #include #include #include @@ -29,6 +28,7 @@ #include "dnsparser.hh" #include "dnsname.hh" #include "dns.hh" +#include "lock.hh" #include "validate.hh" using namespace ::boost::multi_index; @@ -71,17 +71,13 @@ public: void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional capTTD); bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch = false); bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne); - size_t count(const DNSName& qname) const; - size_t count(const DNSName& qname, const QType qtype) const; + size_t count(const DNSName& qname); + size_t count(const DNSName& qname, const QType qtype); void prune(size_t maxEntries); void clear(); - size_t dumpToFile(FILE* fd, const struct timeval& now) const; + size_t dumpToFile(FILE* fd, const struct timeval& now); size_t wipe(const DNSName& name, bool subtree = false); - size_t size() const; - - void preRemoval(const NegCacheEntry& entry) - { - } + size_t size(); private: struct CompositeKey @@ -111,42 +107,36 @@ private: MapCombo(const MapCombo&) = delete; MapCombo& operator=(const MapCombo&) = delete; negcache_t d_map; - mutable std::mutex mutex; std::atomic d_entriesCount{0}; - mutable uint64_t d_contended_count{0}; - mutable uint64_t d_acquired_count{0}; + uint64_t d_contended_count{0}; + uint64_t d_acquired_count{0}; void invalidate() {} }; - vector d_maps; + vector> d_maps; - MapCombo& getMap(const DNSName& qname) + LockGuarded& getMap(const DNSName& qname) { - return d_maps[qname.hash() % d_maps.size()]; + return d_maps.at(qname.hash() % d_maps.size()); } - const MapCombo& getMap(const DNSName& qname) const + const LockGuarded& getMap(const DNSName& qname) const { - return d_maps[qname.hash() % d_maps.size()]; + return d_maps.at(qname.hash() % d_maps.size()); } public: - struct lock + static LockGuardedTryHolder lock(LockGuarded& map) { - lock(const MapCombo& map) : - m(map.mutex) - { - if (!m.try_lock()) { - m.lock(); - map.d_contended_count++; - } - map.d_acquired_count++; - } - ~lock() - { - m.unlock(); + auto locked = map.try_lock(); + if (!locked.owns_lock()) { + locked.lock(); + locked->d_contended_count++; } + ++locked->d_acquired_count; + return locked; + } - private: - std::mutex& m; - }; + void preRemoval(MapCombo& map, const NegCacheEntry& entry) + { + } };