From 8c97604512c046fc96afb9288da17f7edb1dcd89 Mon Sep 17 00:00:00 2001 From: Miod Vallat Date: Thu, 30 Jan 2025 16:40:01 +0100 Subject: [PATCH] Rework domain id generation. These changes make the internal state (TDI_t) the reference for domain ids, which are generated on the fly as domains are added. Routines iterating over domains make sure to keep this state updated, so that, at any time, any domain in existence has an entry in the state and thus an immutable (once discovered) id. While there, implement getDomainInfo() as a wrapper over getAllDomains(), returning only the domain we are looking for. Fixes #14563 --- modules/tinydnsbackend/tinydnsbackend.cc | 117 ++++++++++++++++------- modules/tinydnsbackend/tinydnsbackend.hh | 7 +- 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/modules/tinydnsbackend/tinydnsbackend.cc b/modules/tinydnsbackend/tinydnsbackend.cc index 15c86b3c68..412b6f612a 100644 --- a/modules/tinydnsbackend/tinydnsbackend.cc +++ b/modules/tinydnsbackend/tinydnsbackend.cc @@ -88,46 +88,45 @@ TinyDNSBackend::TinyDNSBackend(const string& suffix) d_isWildcardQuery = false; } +TinyDNSBackend::TDI_t::iterator TinyDNSBackend::updateState(DomainInfo& domain, TDI_t* state) +{ + TDIByZone_t& zone_index = state->get(); + TDIByZone_t::iterator itByZone = zone_index.find(domain.zone); + if (itByZone != zone_index.end()) { + return itByZone; + } + + TinyDomainInfo tmp; + s_lastId++; + tmp.zone = domain.zone; + tmp.id = s_lastId; + tmp.notified_serial = domain.serial; + return state->insert(tmp).first; +} + void TinyDNSBackend::getUpdatedPrimaries(vector& retDomains, std::unordered_set& /* catalogs */, CatalogHashMap& /* catalogHashes */) { + bool alwaysNotify{false}; auto domainInfo = s_domainInfo.lock(); //TODO: We could actually lock less if we do it per suffix. - if (!domainInfo->count(d_suffix)) { + if (domainInfo->count(d_suffix) == 0) { + // If we don't have any state yet, this is startup, check whether we need + // to always notify. + alwaysNotify = mustDo("notify-on-startup"); TDI_t tmp; domainInfo->emplace(d_suffix, tmp); } - TDI_t* domains = &(*domainInfo)[d_suffix]; + TDI_t* state = &(*domainInfo)[d_suffix]; vector allDomains; - getAllDomains(&allDomains, true, false); - if (domains->size() == 0 && !mustDo("notify-on-startup")) { - for (vector::iterator di = allDomains.begin(); di != allDomains.end(); ++di) { - di->notified_serial = 0; - } - } - - for (vector::iterator di = allDomains.begin(); di != allDomains.end(); ++di) { - TDIByZone_t& zone_index = domains->get(); - TDIByZone_t::iterator itByZone = zone_index.find(di->zone); - if (itByZone == zone_index.end()) { - s_lastId++; - - TinyDomainInfo tmp; - tmp.zone = di->zone; - tmp.id = s_lastId; - tmp.notified_serial = di->serial; - domains->insert(tmp); - - di->id = s_lastId; - if (di->notified_serial > 0) { - retDomains.push_back(*di); - } - } - else { - if (itByZone->notified_serial < di->serial) { - di->id = itByZone->id; - retDomains.push_back(*di); - } + getAllDomains_locked(&allDomains, true); + + for (auto& domain : allDomains) { + auto iter = updateState(domain, state); + // Keep domain id in sync with our current state. + domain.id = iter->id; + if (alwaysNotify || iter->notified_serial < domain.serial) { + retDomains.push_back(domain); } } } @@ -138,8 +137,8 @@ void TinyDNSBackend::setNotified(uint32_t id, uint32_t serial) if (!domainInfo->count(d_suffix)) { throw PDNSException("Can't get list of domains to set the serial."); } - TDI_t* domains = &(*domainInfo)[d_suffix]; - TDIById_t& domain_index = domains->get(); + TDI_t* state = &(*domainInfo)[d_suffix]; + TDIById_t& domain_index = state->get(); TDIById_t::iterator itById = domain_index.find(id); if (itById == domain_index.end()) { g_log << Logger::Error << backendname << "Received updated serial(" << serial << "), but domain ID (" << id << ") is not known in this backend." << endl; @@ -148,10 +147,10 @@ void TinyDNSBackend::setNotified(uint32_t id, uint32_t serial) DLOG(g_log << Logger::Debug << backendname << "Setting serial for " << itById->zone << " to " << serial << endl); domain_index.modify(itById, TDI_SerialModifier(serial)); } - (*domainInfo)[d_suffix] = *domains; + (*domainInfo)[d_suffix] = *state; } -void TinyDNSBackend::getAllDomains(vector* domains, bool getSerial, bool /* include_disabled */) +void TinyDNSBackend::getAllDomains_locked(vector* domains, bool getSerial) { d_isAxfr = true; d_isGetDomains = true; @@ -172,7 +171,7 @@ void TinyDNSBackend::getAllDomains(vector* domains, bool getSerial, while (get(rr)) { if (rr.qtype.getCode() == QType::SOA && dupcheck.insert(rr.qname).second) { DomainInfo di; - di.id = -1; //TODO: Check if this is ok. + di.id = -1; // Will be overridden by caller di.backend = this; di.zone = rr.qname; di.kind = DomainInfo::Primary; @@ -195,6 +194,52 @@ void TinyDNSBackend::getAllDomains(vector* domains, bool getSerial, } } +void TinyDNSBackend::getAllDomains(vector* domains, bool getSerial, bool /* include_disabled */) +{ + auto domainInfo = s_domainInfo.lock(); //TODO: We could actually lock less if we do it per suffix. + if (domainInfo->count(d_suffix) == 0) { + TDI_t tmp; + domainInfo->emplace(d_suffix, tmp); + } + + TDI_t* state = &(*domainInfo)[d_suffix]; + + getAllDomains_locked(domains, getSerial); + + for (auto& domain : *domains) { + auto iter = updateState(domain, state); + // Keep domain id in sync with our current state. + domain.id = iter->id; + } +} + +//NOLINTNEXTLINE(readability-identifier-length) +bool TinyDNSBackend::getDomainInfo(const DNSName& domain, DomainInfo& di, bool getSerial) +{ + auto domainInfo = s_domainInfo.lock(); //TODO: We could actually lock less if we do it per suffix. + if (domainInfo->count(d_suffix) == 0) { + TDI_t tmp; + domainInfo->emplace(d_suffix, tmp); + } + + TDI_t* state = &(*domainInfo)[d_suffix]; + + vector allDomains; + getAllDomains_locked(&allDomains, getSerial); + + bool found{false}; + for (auto& oneDomain : allDomains) { + auto iter = updateState(oneDomain, state); + if (oneDomain.zone == domain) { + // Keep domain id in sync with our current state. + oneDomain.id = iter->id; + di = oneDomain; + found = true; + } + } + return found; +} + bool TinyDNSBackend::list(const DNSName& target, int /* domain_id */, bool /* include_disabled */) { d_isAxfr = true; diff --git a/modules/tinydnsbackend/tinydnsbackend.hh b/modules/tinydnsbackend/tinydnsbackend.hh index bcad58d4c3..26bce0784f 100644 --- a/modules/tinydnsbackend/tinydnsbackend.hh +++ b/modules/tinydnsbackend/tinydnsbackend.hh @@ -70,6 +70,7 @@ public: void lookup(const QType& qtype, const DNSName& qdomain, int zoneId, DNSPacket* pkt_p = nullptr) override; bool list(const DNSName& target, int domain_id, bool include_disabled = false) override; bool get(DNSResourceRecord& rr) override; + bool getDomainInfo(const DNSName& domain, DomainInfo& di, bool getSerial = true) override; void getAllDomains(vector* domains, bool getSerial, bool include_disabled) override; // Primary mode operation @@ -77,8 +78,6 @@ public: void setNotified(uint32_t id, uint32_t serial) override; private: - vector getLocations(); - //TypeDefs struct tag_zone { @@ -96,6 +95,10 @@ private: typedef TDI_t::index::type TDIByZone_t; typedef TDI_t::index::type TDIById_t; + vector getLocations(); + static TDI_t::iterator updateState(DomainInfo& domain, TDI_t* state); + void getAllDomains_locked(vector* domains, bool getSerial); + //data member variables uint64_t d_taiepoch; QType d_qtype; -- 2.47.2