From: Otto Moerbeek Date: Mon, 27 May 2024 07:21:01 +0000 (+0200) Subject: Use actual timeout value for nsspeeds; don't throttle on short timeouts X-Git-Tag: rec-5.1.0-beta1~22^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=138b0a25d5aa0c89583399e2bde0a8573b348dad;p=thirdparty%2Fpdns.git Use actual timeout value for nsspeeds; don't throttle on short timeouts --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 268a55d850..95ab36a0d3 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5317,6 +5317,23 @@ void SyncRes::updateQueryCounts(const string& prefix, const DNSName& qname, cons } } +void SyncRes::incTimeoutStats(const ComboAddress& remoteIP) +{ + d_timeouts++; + t_Counters.at(rec::Counter::outgoingtimeouts)++; + + if (remoteIP.sin4.sin_family == AF_INET) { + t_Counters.at(rec::Counter::outgoing4timeouts)++; + } + else { + t_Counters.at(rec::Counter::outgoing6timeouts)++; + } + + if (t_timeouts) { + t_timeouts->push_back(remoteIP); + } +} + bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType qtype, LWResult& lwr, boost::optional& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional& extendedError, bool dontThrottle) { bool chained = false; @@ -5367,22 +5384,8 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, if (resolveret != LWResult::Result::Success) { /* Error while resolving */ if (resolveret == LWResult::Result::Timeout) { - /* Time out */ - LOG(prefix << qname << ": Timeout resolving after " << lwr.d_usec / 1000.0 << " ms " << (doTCP ? "over TCP" : "") << endl); - d_timeouts++; - t_Counters.at(rec::Counter::outgoingtimeouts)++; - - if (remoteIP.sin4.sin_family == AF_INET) { - t_Counters.at(rec::Counter::outgoing4timeouts)++; - } - else { - t_Counters.at(rec::Counter::outgoing6timeouts)++; - } - - if (t_timeouts) { - t_timeouts->push_back(remoteIP); - } + incTimeoutStats(remoteIP); } else if (resolveret == LWResult::Result::OSLimitError) { /* OS resource limit reached */ @@ -5425,15 +5428,25 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, LOG(prefix << qname << ": " << nsName << " (" << remoteIP.toString() << ") returned a packet we could not parse over " << (doTCP ? "TCP" : "UDP") << ", trying sibling IP or NS" << endl); if (!chained && !dontThrottle) { - // let's make sure we prefer a different server for some time, if there is one available - s_nsSpeeds.lock()->find_or_enter(nsName.empty() ? DNSName(remoteIP.toStringWithPort()) : nsName, d_now).submit(remoteIP, 1000000, d_now); // 1 sec - - if (doTCP) { - // we can be more heavy-handed over TCP - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10); + uint32_t responseUsec = 1000000; // 1 sec for non-timeout cases + // Use the actual time if we saw a timeout + if (resolveret == LWResult::Result::Timeout) { + responseUsec = lwr.d_usec; } - else { - doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2); + // let's make sure we prefer a different server for some time, if there is one available + s_nsSpeeds.lock()->find_or_enter(nsName.empty() ? DNSName(remoteIP.toStringWithPort()) : nsName, d_now).submit(remoteIP, static_cast(responseUsec), d_now); + + // If the actual response time was more than 80% of the default timeout, we throttle. On a + // busy rec we reduce the time we are willing to wait for an auth, it is unfair to throttle on + // such a shortened timeout. + if (resolveret != LWResult::Result::Timeout || responseUsec > g_networkTimeoutMsec * 800) { + if (doTCP) { + // we can be more heavy-handed over TCP + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10); + } + else { + doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2); + } } } return false; diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 82bd31ad08..ebf0005e98 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -620,6 +620,7 @@ private: unsigned int depth, const string& prefix, set& beenthere, Context& context, StopAtDelegation* stopAtDelegation, std::map>* fallback); void ednsStats(boost::optional& ednsmask, const DNSName& qname, const string& prefix); + void incTimeoutStats(const ComboAddress& remoteIP); bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, QType qtype, LWResult& lwr, boost::optional& ednsmask, const DNSName& auth, bool sendRDQuery, bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional& extendedError, bool dontThrottle = false); bool processAnswer(unsigned int depth, const string& prefix, LWResult& lwr, const DNSName& qname, QType qtype, DNSName& auth, bool wasForwarded, const boost::optional& ednsmask, bool sendRDQuery, NsSet& nameservers, std::vector& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state, const ComboAddress& remoteIP); diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 4db2c8ab73..d0b5892e16 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -20,10 +20,8 @@ GlobalStateHolder g_DoTToAuthNames; std::unique_ptr g_recCache; std::unique_ptr g_negCache; bool g_lowercaseOutgoing = false; -#if 0 -pdns::TaskQueue g_test_tasks; -pdns::TaskQueue g_resolve_tasks; -#endif +unsigned int g_networkTimeoutMsec = 1500; + /* Fake some required functions we didn't want the trouble to link with */ ArgvMap& arg()