From d16f0c9bde91cabb872580de1bb8d08a39045bcc Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 25 Jul 2025 16:19:43 +0200 Subject: [PATCH] dnsdist: Backend QPS limit refactoring Use `std::optional` to signal that the QPS limit is not set, instead of a relying of a special case version of the `QPSLimiter` that does pretty much nothing. Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-backend.cc | 7 ++++++- pdns/dnsdistdist/dnsdist-lbpolicies.cc | 2 +- pdns/dnsdistdist/dnsdist-lua-bindings.cc | 7 ++++++- pdns/dnsdistdist/dnsdist-lua.cc | 4 ++-- pdns/dnsdistdist/dnsdist-snmp.cc | 2 +- pdns/dnsdistdist/dnsdist-web.cc | 2 +- pdns/dnsdistdist/dnsdist.hh | 8 ++++++-- 7 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc index c93b9e561..47593785d 100644 --- a/pdns/dnsdistdist/dnsdist-backend.cc +++ b/pdns/dnsdistdist/dnsdist-backend.cc @@ -296,7 +296,7 @@ DownstreamState::DownstreamState(DownstreamState::Config&& config, std::shared_p threadStarted.clear(); if (d_config.d_qpsLimit > 0) { - qps = QPSLimiter(d_config.d_qpsLimit, d_config.d_qpsLimit); + d_qpsLimiter = QPSLimiter(d_config.d_qpsLimit, d_config.d_qpsLimit); } if (d_config.id) { @@ -1010,6 +1010,11 @@ bool DownstreamState::parseAvailabilityConfigFromStr(DownstreamState::Config& co return false; } +unsigned int DownstreamState::getQPSLimit() const +{ + return d_qpsLimiter ? d_qpsLimiter->getRate() : 0U; +} + size_t ServerPool::countServers(bool upOnly) { std::shared_ptr servers = nullptr; diff --git a/pdns/dnsdistdist/dnsdist-lbpolicies.cc b/pdns/dnsdistdist/dnsdist-lbpolicies.cc index ac17eb5ef..c8749196f 100644 --- a/pdns/dnsdistdist/dnsdist-lbpolicies.cc +++ b/pdns/dnsdistdist/dnsdist-lbpolicies.cc @@ -75,7 +75,7 @@ shared_ptr leastOutstanding(const ServerPolicy::NumberedServerV shared_ptr firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq) { for (auto& d : servers) { - if (d.second->isUp() && d.second->qps.checkOnly()) { + if (d.second->isUp() && (!d.second->d_qpsLimiter || d.second->d_qpsLimiter->checkOnly())) { return d.second; } } diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings.cc b/pdns/dnsdistdist/dnsdist-lua-bindings.cc index 7f019e752..7d7829aec 100644 --- a/pdns/dnsdistdist/dnsdist-lua-bindings.cc +++ b/pdns/dnsdistdist/dnsdist-lua-bindings.cc @@ -112,7 +112,12 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck) /* DownstreamState */ luaCtx.registerFunction::*)(int)>("setQPS", [](std::shared_ptr& state, int lim) { if (state) { - state->qps = lim > 0 ? QPSLimiter(lim, lim) : QPSLimiter(); + if (lim > 0) { + state->d_qpsLimiter = QPSLimiter(lim, lim); + } + else { + state->d_qpsLimiter.reset(); + } } }); luaCtx.registerFunction::*)(string)>("addPool", [](const std::shared_ptr& state, const string& pool) { diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index f330c2dae..212f52413 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -967,10 +967,10 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) const std::string latency = (backend->latencyUsec == 0.0 ? "-" : boost::str(latFmt % (backend->latencyUsec / 1000.0))); const std::string latencytcp = (backend->latencyUsecTCP == 0.0 ? "-" : boost::str(latFmt % (backend->latencyUsecTCP / 1000.0))); if (showUUIDs) { - ret << (fmt % counter % backend->getName() % backend->d_config.remote.toStringWithPort() % status % backend->queryLoad % backend->qps.getRate() % backend->d_config.order % backend->d_config.d_weight % backend->queries.load() % backend->reuseds.load() % (backend->dropRate) % latency % backend->outstanding.load() % pools % *backend->d_config.id % latencytcp) << endl; + ret << (fmt % counter % backend->getName() % backend->d_config.remote.toStringWithPort() % status % backend->queryLoad % backend->getQPSLimit() % backend->d_config.order % backend->d_config.d_weight % backend->queries.load() % backend->reuseds.load() % (backend->dropRate) % latency % backend->outstanding.load() % pools % *backend->d_config.id % latencytcp) << endl; } else { - ret << (fmt % counter % backend->getName() % backend->d_config.remote.toStringWithPort() % status % backend->queryLoad % backend->qps.getRate() % backend->d_config.order % backend->d_config.d_weight % backend->queries.load() % backend->reuseds.load() % (backend->dropRate) % latency % backend->outstanding.load() % pools % latencytcp) << endl; + ret << (fmt % counter % backend->getName() % backend->d_config.remote.toStringWithPort() % status % backend->queryLoad % backend->getQPSLimit() % backend->d_config.order % backend->d_config.d_weight % backend->queries.load() % backend->reuseds.load() % (backend->dropRate) % latency % backend->outstanding.load() % pools % latencytcp) << endl; } totQPS += static_cast(backend->queryLoad); totQueries += backend->queries.load(); diff --git a/pdns/dnsdistdist/dnsdist-snmp.cc b/pdns/dnsdistdist/dnsdist-snmp.cc index 9d4a05c1f..9064d29d7 100644 --- a/pdns/dnsdistdist/dnsdist-snmp.cc +++ b/pdns/dnsdistdist/dnsdist-snmp.cc @@ -337,7 +337,7 @@ static int backendStatTable_handler(netsnmp_mib_handler* handler, break; case COLUMN_BACKENDQPSLIMIT: DNSDistSNMPAgent::setCounter64Value(request, - server->qps.getRate()); + server->getQPSLimit()); break; case COLUMN_BACKENDREUSED: DNSDistSNMPAgent::setCounter64Value(request, server->reuseds.load()); diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index de657f3ee..061d36e97 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -1092,7 +1092,7 @@ static void addServerToJSON(Json::array& servers, int identifier, const std::sha {"state", status}, {"protocol", backend->getProtocol().toPrettyString()}, {"qps", (double)backend->queryLoad}, - {"qpsLimit", (double)backend->qps.getRate()}, + {"qpsLimit", static_cast(backend->getQPSLimit())}, {"outstanding", (double)backend->outstanding}, {"reuseds", (double)backend->reuseds}, {"weight", (double)backend->d_config.d_weight}, diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh index 3f21e76fe..133186d1b 100644 --- a/pdns/dnsdistdist/dnsdist.hh +++ b/pdns/dnsdistdist/dnsdist.hh @@ -722,7 +722,7 @@ public: std::shared_ptr d_tlsCtx{nullptr}; std::vector sockets; StopWatch sw; - QPSLimiter qps; + std::optional d_qpsLimiter; #ifdef HAVE_XSK std::vector> d_xskInfos; std::vector> d_xskSockets; @@ -874,7 +874,9 @@ public: void incQueriesCount() { ++queries; - qps.addHit(); + if (d_qpsLimiter) { + d_qpsLimiter->addHit(); + } } void incCurrentConnectionsCount(); @@ -932,6 +934,8 @@ public: } return latencyUsec; } + + unsigned int getQPSLimit() const; }; void responderThread(std::shared_ptr dss); -- 2.47.3