Skip to content

Commit 1e12511

Browse files
authored
Merge pull request PowerDNS#14408 from omoerbeek/rec-throttle-reason
Rec throttle reason
2 parents 313155d + 1fa2c84 commit 1e12511

File tree

4 files changed

+94
-34
lines changed

4 files changed

+94
-34
lines changed

.not-formatted

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
./pdns/auth-carbon.cc
99
./pdns/auth-packetcache.cc
1010
./pdns/auth-packetcache.hh
11+
./pdns/auth-primarycommunicator.cc
1112
./pdns/auth-querycache.cc
1213
./pdns/auth-querycache.hh
1314
./pdns/axfr-retriever.cc
@@ -125,7 +126,6 @@
125126
./pdns/lua-record.cc
126127
./pdns/malloctrace.cc
127128
./pdns/malloctrace.hh
128-
./pdns/auth-primarycommunicator.cc
129129
./pdns/minicurl.cc
130130
./pdns/minicurl.hh
131131
./pdns/misc.cc

pdns/recursordist/syncres.cc

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -245,26 +245,36 @@ class nsspeeds_t : public multi_index_container<DecayingEwmaCollection,
245245

246246
static LockGuarded<nsspeeds_t> s_nsSpeeds;
247247

248-
template <class Thing>
249-
class Throttle : public boost::noncopyable
248+
class Throttle
250249
{
251250
public:
251+
Throttle() = default;
252+
~Throttle() = default;
253+
Throttle(Throttle&&) = delete;
254+
Throttle& operator=(const Throttle&) = default;
255+
Throttle& operator=(Throttle&&) = delete;
256+
Throttle(const Throttle&) = delete;
257+
258+
using Key = std::tuple<ComboAddress, DNSName, QType>;
259+
using Reason = SyncRes::ThrottleReason;
260+
252261
struct entry_t
253262
{
254-
entry_t(const Thing& thing_, time_t ttd_, unsigned int count_) :
255-
thing(thing_), ttd(ttd_), count(count_)
263+
entry_t(Key thing_, time_t ttd_, unsigned int count_, Reason reason_) :
264+
thing(std::move(thing_)), ttd(ttd_), count(count_), reason(reason_)
256265
{
257266
}
258-
Thing thing;
267+
Key thing;
259268
time_t ttd;
260269
mutable unsigned int count;
270+
Reason reason;
261271
};
262272
using cont_t = multi_index_container<entry_t,
263273
indexed_by<
264-
ordered_unique<tag<Thing>, member<entry_t, Thing, &entry_t::thing>>,
274+
ordered_unique<tag<Key>, member<entry_t, Key, &entry_t::thing>>,
265275
ordered_non_unique<tag<time_t>, member<entry_t, time_t, &entry_t::ttd>>>>;
266276

267-
bool shouldThrottle(time_t now, const Thing& arg)
277+
bool shouldThrottle(time_t now, const Key& arg)
268278
{
269279
auto iter = d_cont.find(arg);
270280
if (iter == d_cont.end()) {
@@ -279,18 +289,22 @@ class Throttle : public boost::noncopyable
279289
return true; // still listed, still blocked
280290
}
281291

282-
void throttle(time_t now, const Thing& arg, time_t ttl, unsigned int count)
292+
void throttle(time_t now, const Key& arg, time_t ttl, unsigned int count, Reason reason)
283293
{
284294
auto iter = d_cont.find(arg);
285295
time_t ttd = now + ttl;
286296
if (iter == d_cont.end()) {
287-
d_cont.emplace(arg, ttd, count);
297+
d_cont.emplace(arg, ttd, count, reason);
288298
}
289299
else if (ttd > iter->ttd || count > iter->count) {
290300
ttd = std::max(iter->ttd, ttd);
291301
count = std::max(iter->count, count);
292-
auto& ind = d_cont.template get<Thing>();
293-
ind.modify(iter, [ttd, count](entry_t& entry) { entry.ttd = ttd; entry.count = count; });
302+
auto& ind = d_cont.template get<Key>();
303+
ind.modify(iter, [ttd, count, reason](entry_t& entry) {
304+
entry.ttd = ttd;
305+
entry.count = count;
306+
entry.reason = reason;
307+
});
294308
}
295309
}
296310

@@ -309,7 +323,7 @@ class Throttle : public boost::noncopyable
309323
d_cont.clear();
310324
}
311325

312-
void clear(const Thing& thing)
326+
void clear(const Key& thing)
313327
{
314328
d_cont.erase(thing);
315329
}
@@ -319,11 +333,31 @@ class Throttle : public boost::noncopyable
319333
ind.erase(ind.begin(), ind.upper_bound(now));
320334
}
321335

336+
static std::string toString(Reason reason)
337+
{
338+
static const std::array<std::string, 10> reasons = {
339+
"None",
340+
"ServerDown",
341+
"PermanentError",
342+
"Timeout",
343+
"ParseError",
344+
"RCodeServFail",
345+
"RCodeRefused",
346+
"RCodeOther",
347+
"TCPTruncate",
348+
"Lame"};
349+
const auto index = static_cast<unsigned int>(reason);
350+
if (index >= reasons.size()) {
351+
return "?";
352+
}
353+
return reasons.at(index);
354+
}
355+
322356
private:
323357
cont_t d_cont;
324358
};
325359

326-
static LockGuarded<Throttle<std::tuple<ComboAddress, DNSName, QType>>> s_throttle;
360+
static LockGuarded<Throttle> s_throttle;
327361

328362
struct SavedParentEntry
329363
{
@@ -1279,14 +1313,14 @@ void SyncRes::unThrottle(const ComboAddress& server, const DNSName& name, QType
12791313
s_throttle.lock()->clear(std::tuple(server, name, qtype));
12801314
}
12811315

1282-
void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries)
1316+
void SyncRes::doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, Throttle::Reason reason)
12831317
{
1284-
s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries);
1318+
s_throttle.lock()->throttle(now, std::tuple(server, g_rootdnsname, 0), duration, tries, reason);
12851319
}
12861320

1287-
void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries)
1321+
void SyncRes::doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, Throttle::Reason reason)
12881322
{
1289-
s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries);
1323+
s_throttle.lock()->throttle(now, std::tuple(server, name, qtype), duration, tries, reason);
12901324
}
12911325

12921326
uint64_t SyncRes::doDumpThrottleMap(int fileDesc)
@@ -1301,16 +1335,16 @@ uint64_t SyncRes::doDumpThrottleMap(int fileDesc)
13011335
return 0;
13021336
}
13031337
fprintf(filePtr.get(), "; throttle map dump follows\n");
1304-
fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\n");
1338+
fprintf(filePtr.get(), "; remote IP\tqname\tqtype\tcount\tttd\treason\n");
13051339
uint64_t count = 0;
13061340

13071341
// Get a copy to avoid holding the lock while doing I/O
13081342
const auto throttleMap = s_throttle.lock()->getThrottleMap();
13091343
for (const auto& iter : throttleMap) {
13101344
count++;
13111345
timebuf_t tmp;
1312-
// remote IP, dns name, qtype, count, ttd
1313-
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));
1346+
// remote IP, dns name, qtype, count, ttd, reason
1347+
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());
13141348
}
13151349

13161350
return count;
@@ -5415,19 +5449,19 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
54155449
if (s_serverdownmaxfails > 0 && auth != g_rootdnsname && s_fails.lock()->incr(remoteIP, d_now) >= s_serverdownmaxfails) {
54165450
LOG(prefix << qname << ": Max fails reached resolving on " << remoteIP.toString() << ". Going full throttle for " << s_serverdownthrottletime << " seconds" << endl);
54175451
// mark server as down
5418-
doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000);
5452+
doThrottle(d_now.tv_sec, remoteIP, s_serverdownthrottletime, 10000, Throttle::Reason::ServerDown);
54195453
}
54205454
else if (resolveret == LWResult::Result::PermanentError) {
54215455
// unreachable, 1 minute or 100 queries
5422-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100);
5456+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 100, Throttle::Reason::PermanentError);
54235457
}
54245458
else {
54255459
// If the actual response time was more than 80% of the default timeout, we throttle. On a
54265460
// busy rec we reduce the time we are willing to wait for an auth, it is unfair to throttle on
54275461
// such a shortened timeout.
54285462
if (responseUsec > g_networkTimeoutMsec * 800) {
54295463
// timeout, 10 seconds or 5 queries
5430-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5);
5464+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 5, Throttle::Reason::Timeout);
54315465
}
54325466
}
54335467
}
@@ -5444,10 +5478,10 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
54445478

54455479
if (doTCP) {
54465480
// we can be more heavy-handed over TCP
5447-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10);
5481+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 10, Throttle::Reason::ParseError);
54485482
}
54495483
else {
5450-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2);
5484+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 10, 2, Throttle::Reason::ParseError);
54515485
}
54525486
}
54535487
return false;
@@ -5463,7 +5497,19 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
54635497
s_nsSpeeds.lock()->find_or_enter(nsName.empty() ? DNSName(remoteIP.toStringWithPort()) : nsName, d_now).submit(remoteIP, 1000000, d_now); // 1 sec
54645498
}
54655499
else {
5466-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3);
5500+
Throttle::Reason reason{};
5501+
switch (lwr.d_rcode) {
5502+
case RCode::ServFail:
5503+
reason = Throttle::Reason::RCodeServFail;
5504+
break;
5505+
case RCode::Refused:
5506+
reason = Throttle::Reason::RCodeRefused;
5507+
break;
5508+
default:
5509+
reason = Throttle::Reason::RCodeOther;
5510+
break;
5511+
}
5512+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, reason);
54675513
}
54685514
}
54695515
return false;
@@ -5483,7 +5529,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
54835529
LOG(prefix << qname << ": Truncated bit set, over TCP?" << endl);
54845530
if (!dontThrottle) {
54855531
/* let's treat that as a ServFail answer from this server */
5486-
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3);
5532+
doThrottle(d_now.tv_sec, remoteIP, qname, qtype, 60, 3, Throttle::Reason::TCPTruncate);
54875533
}
54885534
return false;
54895535
}
@@ -5889,7 +5935,7 @@ int SyncRes::doResolveAt(NsSet& nameservers, DNSName auth, bool flawedNSSet, con
58895935
}
58905936
/* was lame */
58915937
if (!shouldNotThrottle(&tns->first, &*remoteIP)) {
5892-
doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100);
5938+
doThrottle(d_now.tv_sec, *remoteIP, qname, qtype, 60, 100, Throttle::Reason::Lame);
58935939
}
58945940
}
58955941

pdns/recursordist/syncres.hh

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,22 @@ public:
255255
static void clearThrottle();
256256
static bool isThrottled(time_t now, const ComboAddress& server, const DNSName& target, QType qtype);
257257
static bool isThrottled(time_t now, const ComboAddress& server);
258-
static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries);
259-
static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries);
258+
259+
enum class ThrottleReason : uint8_t
260+
{
261+
None,
262+
ServerDown,
263+
PermanentError,
264+
Timeout,
265+
ParseError,
266+
RCodeServFail,
267+
RCodeRefused,
268+
RCodeOther,
269+
TCPTruncate,
270+
Lame,
271+
};
272+
static void doThrottle(time_t now, const ComboAddress& server, time_t duration, unsigned int tries, ThrottleReason reason);
273+
static void doThrottle(time_t now, const ComboAddress& server, const DNSName& name, QType qtype, time_t duration, unsigned int tries, ThrottleReason reason);
260274
static void unThrottle(const ComboAddress& server, const DNSName& qname, QType qtype);
261275

262276
static uint64_t getFailedServersSize();

pdns/recursordist/test-syncres_cc2.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server)
410410

411411
/* mark ns as down */
412412
time_t now = sr->getNow().tv_sec;
413-
SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000);
413+
SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000, SyncRes::ThrottleReason::Timeout);
414414

415415
vector<DNSRecord> ret;
416416
int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
@@ -432,7 +432,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_count)
432432
const size_t blocks = 10;
433433
/* mark ns as down for 'blocks' queries */
434434
time_t now = sr->getNow().tv_sec;
435-
SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks);
435+
SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks, SyncRes::ThrottleReason::Timeout);
436436

437437
for (size_t idx = 0; idx < blocks; idx++) {
438438
BOOST_CHECK(SyncRes::isThrottled(now, ns));
@@ -454,7 +454,7 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_time)
454454
const size_t seconds = 1;
455455
/* mark ns as down for 'seconds' seconds */
456456
time_t now = sr->getNow().tv_sec;
457-
SyncRes::doThrottle(now, ns, seconds, 10000);
457+
SyncRes::doThrottle(now, ns, seconds, 10000, SyncRes::ThrottleReason::Timeout);
458458

459459
BOOST_CHECK(SyncRes::isThrottled(now, ns));
460460

0 commit comments

Comments
 (0)