From: Otto Moerbeek Date: Fri, 28 Jun 2024 14:01:51 +0000 (+0200) Subject: Remember reason we're throttling X-Git-Tag: rec-5.2.0-alpha1~201^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=36dcac3b04f2cf0177e0342c05ffb53183381b1f;p=thirdparty%2Fpdns.git Remember reason we're throttling --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index b504f66fd4..1e5864aaa9 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -256,15 +256,18 @@ public: Throttle(const Throttle&) = delete; using Key = std::tuple; + using Reason = SyncRes::ThrottleReason; + struct entry_t { - entry_t(Key thing_, time_t ttd_, unsigned int count_) : - thing(std::move(thing_)), ttd(ttd_), count(count_) + entry_t(Key thing_, time_t ttd_, unsigned int count_, Reason reason_) : + thing(std::move(thing_)), ttd(ttd_), count(count_), reason(reason_) { } Key thing; time_t ttd; mutable unsigned int count; + Reason reason; }; using cont_t = multi_index_container iter->ttd || count > iter->count) { ttd = std::max(iter->ttd, ttd); count = std::max(iter->count, count); auto& ind = d_cont.template get(); - ind.modify(iter, [ttd, count](entry_t& entry) { entry.ttd = ttd; entry.count = count; }); + ind.modify(iter, [ttd, count, reason](entry_t& entry) { + entry.ttd = ttd; + entry.count = count; + entry.reason = reason; + }); } } @@ -326,6 +333,26 @@ public: ind.erase(ind.begin(), ind.upper_bound(now)); } + static std::string toString(Reason reason) + { + static const std::array reasons = { + "None", + "ServerDown", + "PermanentError", + "Timeout", + "ParseError", + "RCodeServFail", + "RCodeRefused", + "RCodeOther", + "TCPTruncate", + "Lame"}; + const auto index = static_cast(reason); + if (index >= reasons.size()) { + return "?"; + } + return reasons.at(index); + } + private: cont_t d_cont; }; @@ -1286,14 +1313,14 @@ void SyncRes::unThrottle(const ComboAddress& server, const DNSName& name, QType s_throttle.lock()->clear(std::tuple(server, name, qtype)); } -void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries) +void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, Throttle::Reason reason) { - s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries); + s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries, reason); } -void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries) +void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, Throttle::Reason reason) { - s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries); + s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries, reason); } uint64_t SyncRes::doDumpThrottleMap(int fileDesc) @@ -1308,7 +1335,7 @@ uint64_t SyncRes::doDumpThrottleMap(int fileDesc) return 0; } fprintf(filePtr.get(), "; throttle map dump follows\n"); - fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\n"); + fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\treason\n"); uint64_t count = 0; // Get a copy to avoid holding the lock while doing I/O @@ -1316,8 +1343,8 @@ uint64_t SyncRes::doDumpThrottleMap(int fileDesc) for (const auto& iter : throttleMap) { count++; timebuf_t tmp; - // remote IP, dns name, qtype, count, ttd - fprintf(filePtr.get(), "%s\t%s\t%s\t%u\t%s\n", std::get<0>(iter.thing).toString().c_str(), std::get<1>(iter.thing).toLogString().c_str(), std::get<2>(iter.thing).toString().c_str(), iter.count, timestamp(iter.ttd, tmp)); + // remote IP, dns name, qtype, count, ttd, reason + fprintf(filePtr.get(), "%s\t%s\t%s\t%u\t%s\t%s\n", std::get<0>(iter.thing).toString().c_str(), std::get<1>(iter.thing).toLogString().c_str(), std::get<2>(iter.thing).toString().c_str(), iter.count, timestamp(iter.ttd, tmp), Throttle::toString(iter.reason).c_str()); } return count; @@ -5422,11 +5449,11 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, if (s_serverdownmaxfails > 0 && auth != g_rootdnsname && s_fails.lock()->incr(remoteIP, d_now) >= s_serverdownmaxfails) { LOG(prefix << qname << ": Max fails reached resolving on " << remoteIP.toString() << ". Going full throttle for " << s_serverdownthrottletime << " seconds" << endl); // mark server as down - doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000); + doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000, Throttle::Reason::ServerDown); } else if (resolveret == LWResult::Result::PermanentError) { // unreachable, 1 minute or 100 queries - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100); + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100, Throttle::Reason::PermanentError); } else { // If the actual response time was more than 80% of the default timeout, we throttle. On a @@ -5434,7 +5461,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, // such a shortened timeout. if (responseUsec > g_networkTimeoutMsec * 800) { // timeout, 10 seconds or 5 queries - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5); + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5, Throttle::Reason::Timeout); } } } @@ -5451,10 +5478,10 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, if (doTCP) { // we can be more heavy-handed over TCP - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10); + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10, Throttle::Reason::ParseError); } else { - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2); + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2, Throttle::Reason::ParseError); } } return false; @@ -5470,7 +5497,19 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, s_nsSpeeds.lock()->find_or_enter(nsName.empty() ? DNSName(remoteIP.toStringWithPort()) : nsName, d_now).submit(remoteIP, 1000000, d_now); // 1 sec } else { - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3); + Throttle::Reason reason{}; + switch (lwr.d_rcode) { + case RCode::ServFail: + reason = Throttle::Reason::RCodeServFail; + break; + case RCode::Refused: + reason = Throttle::Reason::RCodeRefused; + break; + default: + reason = Throttle::Reason::RCodeOther; + break; + } + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, reason); } } return false; @@ -5490,7 +5529,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, LOG(prefix << qname << ": Truncated bit set, over TCP?" << endl); if (!dontThrottle) { /* let's treat that as a ServFail answer from this server */ - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3); + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, Throttle::Reason::TCPTruncate); } return false; } @@ -5896,7 +5935,7 @@ int SyncRes::doResolveAt(NsSet& nameservers, DNSName auth, bool flawedNSSet, con } /* was lame */ if (!shouldNotThrottle(&tns->first, &*remoteIP)) { - doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100); + doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100, Throttle::Reason::Lame); } } diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index ebf0005e98..09db4ac976 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -255,8 +255,22 @@ public: static void clearThrottle(); static bool isThrottled(time_t now, const ComboAddress& server, const DNSName& target, QType qtype); static bool isThrottled(time_t now, const ComboAddress& server); - static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries); - static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries); + + enum class ThrottleReason : uint8_t + { + None, + ServerDown, + PermanentError, + Timeout, + ParseError, + RCodeServFail, + RCodeRefused, + RCodeOther, + TCPTruncate, + Lame, + }; + static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, ThrottleReason reason); + static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, ThrottleReason reason); static void unThrottle(const ComboAddress& server, const DNSName& qname, QType qtype); static uint64_t getFailedServersSize(); @@ -894,7 +908,7 @@ class ImmediateServFailException { public: ImmediateServFailException(string reason_) : - reason(std::move(reason_)){}; + reason(std::move(reason_)) {}; string reason; //! Print this to tell the user what went wrong }; diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc index 7f57813e6b..dc0fc76d58 100644 --- a/pdns/recursordist/test-syncres_cc2.cc +++ b/pdns/recursordist/test-syncres_cc2.cc @@ -410,7 +410,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server) /* mark ns as down */ time_t now = sr->getNow().tv_sec; - SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000); + SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000, SyncRes::ThrottleReason::Timeout); vector ret; int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); @@ -432,7 +432,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_count) const size_t blocks = 10; /* mark ns as down for 'blocks' queries */ time_t now = sr->getNow().tv_sec; - SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks); + SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks, SyncRes::ThrottleReason::Timeout); for (size_t idx = 0; idx < blocks; idx++) { BOOST_CHECK(SyncRes::isThrottled(now, ns)); @@ -454,7 +454,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_time) const size_t seconds = 1; /* mark ns as down for 'seconds' seconds */ time_t now = sr->getNow().tv_sec; - SyncRes::doThrottle(now, ns, seconds, 10000); + SyncRes::doThrottle(now, ns, seconds, 10000, SyncRes::ThrottleReason::Timeout); BOOST_CHECK(SyncRes::isThrottled(now, ns));