]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Server pools are no longer ref counted
authorRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 18 Jul 2025 08:15:36 +0000 (10:15 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 6 Oct 2025 14:50:22 +0000 (16:50 +0200)
Since the refactoring of the runtime configuration, the content of
a Server Pool is now in effect immutable, we have to create a new
copy and update it, so we no longer have to lock and reference count
Server Pools and their content.

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>
16 files changed:
pdns/dnsdistdist/dnsdist-backend.cc
pdns/dnsdistdist/dnsdist-carbon.cc
pdns/dnsdistdist/dnsdist-configuration-yaml.cc
pdns/dnsdistdist/dnsdist-configuration.hh
pdns/dnsdistdist/dnsdist-lbpolicies.cc
pdns/dnsdistdist/dnsdist-lbpolicies.hh
pdns/dnsdistdist/dnsdist-lua-bindings.cc
pdns/dnsdistdist/dnsdist-lua-ffi.cc
pdns/dnsdistdist/dnsdist-lua.cc
pdns/dnsdistdist/dnsdist-lua.hh
pdns/dnsdistdist/dnsdist-rules-factory.hh
pdns/dnsdistdist/dnsdist-server-pool.hh [new file with mode: 0644]
pdns/dnsdistdist/dnsdist-web.cc
pdns/dnsdistdist/dnsdist.cc
pdns/dnsdistdist/dnsdist.hh
pdns/dnsdistdist/test-dnsdist-lua-ffi.cc

index fa44589457e144ca507874f637a87ec1823e9843..ad8d7fde31a2fac39ecc293c018f502409949d13 100644 (file)
@@ -1015,16 +1015,10 @@ unsigned int DownstreamState::getQPSLimit() const
   return d_qpsLimiter ? d_qpsLimiter->getRate() : 0U;
 }
 
-size_t ServerPool::countServers(bool upOnly)
+size_t ServerPool::countServers(bool upOnly) const
 {
-  std::shared_ptr<const ServerPolicy::NumberedServerVector> servers = nullptr;
-  {
-    auto lock = d_servers.read_lock();
-    servers = *lock;
-  }
-
   size_t count = 0;
-  for (const auto& server : *servers) {
+  for (const auto& server : d_servers) {
     if (!upOnly || std::get<1>(server)->isUp() ) {
       count++;
     }
@@ -1033,51 +1027,36 @@ size_t ServerPool::countServers(bool upOnly)
   return count;
 }
 
-size_t ServerPool::poolLoad()
+size_t ServerPool::poolLoad() const
 {
-  std::shared_ptr<const ServerPolicy::NumberedServerVector> servers = nullptr;
-  {
-    auto lock = d_servers.read_lock();
-    servers = *lock;
-  }
-
   size_t load = 0;
-  for (const auto& server : *servers) {
+  for (const auto& server : d_servers) {
     size_t serverOutstanding = std::get<1>(server)->outstanding.load();
     load += serverOutstanding;
   }
   return load;
 }
 
-const std::shared_ptr<const ServerPolicy::NumberedServerVector> ServerPool::getServers()
+const ServerPolicy::NumberedServerVector& ServerPool::getServers() const
 {
-  std::shared_ptr<const ServerPolicy::NumberedServerVector> result;
-  {
-    result = *(d_servers.read_lock());
-  }
-  return result;
+  return d_servers;
 }
 
-void ServerPool::addServer(shared_ptr<DownstreamState>& server)
+void ServerPool::addServer(std::shared_ptr<DownstreamState>& server)
 {
-  auto servers = d_servers.write_lock();
-  /* we can't update the content of the shared pointer directly even when holding the lock,
-     as other threads might hold a copy. We can however update the pointer as long as we hold the lock. */
-  unsigned int count = static_cast<unsigned int>((*servers)->size());
-  auto newServers = ServerPolicy::NumberedServerVector(*(*servers));
-  newServers.emplace_back(++count, server);
+  unsigned int count = static_cast<unsigned int>(d_servers.size());
+  d_servers.emplace_back(++count, server);
   /* we need to reorder based on the server 'order' */
-  std::stable_sort(newServers.begin(), newServers.end(), [](const std::pair<unsigned int,std::shared_ptr<DownstreamState> >& a, const std::pair<unsigned int,std::shared_ptr<DownstreamState> >& b) {
+  std::stable_sort(d_servers.begin(), d_servers.end(), [](const std::pair<unsigned int,std::shared_ptr<DownstreamState> >& a, const std::pair<unsigned int,std::shared_ptr<DownstreamState> >& b) {
       return a.second->d_config.order < b.second->d_config.order;
     });
   /* and now we need to renumber for Lua (custom policies) */
   size_t idx = 1;
-  for (auto& serv : newServers) {
+  for (auto& serv : d_servers) {
     serv.first = idx++;
   }
-  *servers = std::make_shared<const ServerPolicy::NumberedServerVector>(std::move(newServers));
 
-  if ((*servers)->size() == 1) {
+  if (d_servers.size() == 1) {
     d_tcpOnly = server->isTCPOnly();
   }
   else if (!server->isTCPOnly()) {
@@ -1087,14 +1066,10 @@ void ServerPool::addServer(shared_ptr<DownstreamState>& server)
 
 void ServerPool::removeServer(shared_ptr<DownstreamState>& server)
 {
-  auto servers = d_servers.write_lock();
-  /* we can't update the content of the shared pointer directly even when holding the lock,
-     as other threads might hold a copy. We can however update the pointer as long as we hold the lock. */
-  auto newServers = std::make_shared<ServerPolicy::NumberedServerVector>(*(*servers));
   size_t idx = 1;
   bool found = false;
   bool tcpOnly = true;
-  for (auto it = newServers->begin(); it != newServers->end();) {
+  for (auto it = d_servers.begin(); it != d_servers.end();) {
     if (found) {
       tcpOnly = tcpOnly && it->second->isTCPOnly();
       /* we need to renumber the servers placed
@@ -1103,7 +1078,7 @@ void ServerPool::removeServer(shared_ptr<DownstreamState>& server)
       it++;
     }
     else if (it->second == server) {
-      it = newServers->erase(it);
+      it = d_servers.erase(it);
       found = true;
     } else {
       tcpOnly = tcpOnly && it->second->isTCPOnly();
@@ -1112,7 +1087,6 @@ void ServerPool::removeServer(shared_ptr<DownstreamState>& server)
     }
   }
   d_tcpOnly = tcpOnly;
-  *servers = std::move(newServers);
 }
 
 namespace dnsdist::backend
index 98f254f1435bd3d7830aa07420c988b488d1b91b..82ae493504c4e49c308a0adce702054a9571b1bf 100644 (file)
@@ -194,13 +194,13 @@ static bool doOneCarbonExport(const Carbon::Endpoint& endpoint)
       base += ".pools.";
       base += poolName;
       base += ".";
-      const std::shared_ptr<ServerPool> pool = entry.second;
+      const ServerPool& pool = entry.second;
       str << base << "servers"
-          << " " << pool->countServers(false) << " " << now << "\r\n";
+          << " " << pool.countServers(false) << " " << now << "\r\n";
       str << base << "servers-up"
-          << " " << pool->countServers(true) << " " << now << "\r\n";
-      if (pool->packetCache != nullptr) {
-        const auto& cache = pool->packetCache;
+          << " " << pool.countServers(true) << " " << now << "\r\n";
+      if (pool.packetCache != nullptr) {
+        const auto& cache = pool.packetCache;
         str << base << "cache-size"
             << " " << cache->getMaxEntries() << " " << now << "\r\n";
         str << base << "cache-entries"
index e2b36bc2c88d1b1bef450082dd4291f2245a81b2..013854e43027bd95d3bb520422d380105e45f720 100644 (file)
@@ -1215,13 +1215,19 @@ bool loadConfigurationFromFile(const std::string& fileName, [[maybe_unused]] boo
     }
 
     for (const auto& pool : globalConfig.pools) {
-      std::shared_ptr<ServerPool> poolObj = createPoolIfNotExists(std::string(pool.name));
-      if (!pool.packet_cache.empty()) {
-        poolObj->packetCache = getRegisteredTypeByName<DNSDistPacketCache>(pool.packet_cache);
-      }
-      if (!pool.policy.empty()) {
-        poolObj->policy = getRegisteredTypeByName<ServerPolicy>(pool.policy);
-      }
+      dnsdist::configuration::updateRuntimeConfiguration([&pool](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto [poolIt, inserted] = config.d_pools.emplace(std::string(pool.name), ServerPool());
+        if (inserted) {
+          vinfolog("Creating pool %s", pool.name);
+        }
+
+        if (!pool.packet_cache.empty()) {
+          poolIt->second.packetCache = getRegisteredTypeByName<DNSDistPacketCache>(pool.packet_cache);
+        }
+        if (!pool.policy.empty()) {
+          poolIt->second.policy = getRegisteredTypeByName<ServerPolicy>(pool.policy);
+        }
+      });
     }
 
     loadRulesConfiguration(globalConfig);
index a60a5068670e13bed7764085d1da10a35b112d0d..605d735278fb2b9315fad739a88683b99138285b 100644 (file)
@@ -33,6 +33,7 @@
 #include "dnsdist-carbon.hh"
 #include "dnsdist-query-count.hh"
 #include "dnsdist-rule-chains.hh"
+#include "dnsdist-server-pool.hh"
 #include "iputils.hh"
 
 class ServerPolicy;
@@ -118,7 +119,7 @@ struct RuntimeConfiguration
 #ifndef DISABLE_CARBON
   std::vector<dnsdist::Carbon::Endpoint> d_carbonEndpoints;
 #endif /* DISABLE_CARBON */
-  std::unordered_map<std::string, std::shared_ptr<ServerPool>> d_pools;
+  std::unordered_map<std::string, ServerPool> d_pools;
   std::shared_ptr<const CredentialsHolder> d_webPassword;
   std::shared_ptr<const CredentialsHolder> d_webAPIKey;
   std::optional<std::unordered_map<std::string, std::string>> d_webCustomHeaders;
index c8749196fef3e84597a1a255caf3fde91ce75347..58075c41a50ef4388a47d6bb108fa4a24cf3da8a 100644 (file)
@@ -289,13 +289,12 @@ shared_ptr<DownstreamState> orderedWrandUntag(const ServerPolicy::NumberedServer
   return wrandom(candidates, dnsq);
 }
 
-std::shared_ptr<const ServerPolicy::NumberedServerVector> getDownstreamCandidates(const std::string& poolName)
+const ServerPolicy::NumberedServerVector& getDownstreamCandidates(const std::string& poolName)
 {
-  std::shared_ptr<ServerPool> pool = getPool(poolName);
-  return pool->getServers();
+  return getPool(poolName).getServers();
 }
 
-std::shared_ptr<ServerPool> createPoolIfNotExists(const string& poolName)
+const ServerPool& createPoolIfNotExists(const string& poolName)
 {
   {
     const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
@@ -309,40 +308,47 @@ std::shared_ptr<ServerPool> createPoolIfNotExists(const string& poolName)
     vinfolog("Creating pool %s", poolName);
   }
 
-  auto pool = std::make_shared<ServerPool>();
-  dnsdist::configuration::updateRuntimeConfiguration([&poolName,&pool](dnsdist::configuration::RuntimeConfiguration& config) {
-    config.d_pools.emplace(poolName, pool);
+  dnsdist::configuration::updateRuntimeConfiguration([&poolName](dnsdist::configuration::RuntimeConfiguration& config) {
+    config.d_pools.emplace(poolName, ServerPool());
   });
 
-  return pool;
+  {
+    const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
+    const auto poolIt = pools.find(poolName);
+    return poolIt->second;
+  }
 }
 
 void setPoolPolicy(const string& poolName, std::shared_ptr<ServerPolicy> policy)
 {
-  std::shared_ptr<ServerPool> pool = createPoolIfNotExists(poolName);
   if (!poolName.empty()) {
     vinfolog("Setting pool %s server selection policy to %s", poolName, policy->getName());
   } else {
     vinfolog("Setting default pool server selection policy to %s", policy->getName());
   }
-  pool->policy = std::move(policy);
+
+  dnsdist::configuration::updateRuntimeConfiguration([&poolName, &policy](dnsdist::configuration::RuntimeConfiguration& config) {
+    auto [poolIt, inserted] = config.d_pools.emplace(poolName, ServerPool());
+    poolIt->second.policy = std::move(policy);
+  });
 }
 
 void addServerToPool(const string& poolName, std::shared_ptr<DownstreamState> server)
 {
-  std::shared_ptr<ServerPool> pool = createPoolIfNotExists(poolName);
   if (!poolName.empty()) {
     vinfolog("Adding server to pool %s", poolName);
   } else {
     vinfolog("Adding server to default pool");
   }
-  pool->addServer(server);
+
+  dnsdist::configuration::updateRuntimeConfiguration([&poolName, &server](dnsdist::configuration::RuntimeConfiguration& config) {
+    auto [poolIt, inserted] = config.d_pools.emplace(poolName, ServerPool());
+    poolIt->second.addServer(server);
+  });
 }
 
 void removeServerFromPool(const string& poolName, std::shared_ptr<DownstreamState> server)
 {
-  std::shared_ptr<ServerPool> pool = getPool(poolName);
-
   if (!poolName.empty()) {
     vinfolog("Removing server from pool %s", poolName);
   }
@@ -350,10 +356,13 @@ void removeServerFromPool(const string& poolName, std::shared_ptr<DownstreamStat
     vinfolog("Removing server from default pool");
   }
 
-  pool->removeServer(server);
+  dnsdist::configuration::updateRuntimeConfiguration([&poolName, &server](dnsdist::configuration::RuntimeConfiguration& config) {
+    auto [poolIt, inserted] = config.d_pools.emplace(poolName, ServerPool());
+    poolIt->second.removeServer(server);
+  });
 }
 
-std::shared_ptr<ServerPool> getPool(const std::string& poolName)
+const ServerPool& getPool(const std::string& poolName)
 {
   const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
   auto poolIt = pools.find(poolName);
index 291fa754ef4c1de0b033f055b37e5e2916be2184..a6ea89f706a396882fe60f63884b53b28836238a 100644 (file)
@@ -92,13 +92,13 @@ public:
 struct ServerPool;
 
 using pools_t = std::map<std::string, std::shared_ptr<ServerPool>>;
-std::shared_ptr<ServerPool> getPool(const std::string& poolName);
-std::shared_ptr<ServerPool> createPoolIfNotExists(const string& poolName);
+const ServerPool& getPool(const std::string& poolName);
+const ServerPool& createPoolIfNotExists(const string& poolName);
 void setPoolPolicy(const string& poolName, std::shared_ptr<ServerPolicy> policy);
 void addServerToPool(const string& poolName, std::shared_ptr<DownstreamState> server);
 void removeServerFromPool(const string& poolName, std::shared_ptr<DownstreamState> server);
 
-std::shared_ptr<const ServerPolicy::NumberedServerVector> getDownstreamCandidates(const std::string& poolName);
+const ServerPolicy::NumberedServerVector& getDownstreamCandidates(const std::string& poolName);
 
 std::shared_ptr<DownstreamState> firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq);
 
index 033ffe4e4a74f615a7cb558dec0b1308ccb070d1..ca14401186118c9b1c681b1e9ac242a790bb49aa 100644 (file)
@@ -94,19 +94,68 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck)
 #endif /* DISABLE_POLICIES_BINDINGS */
 
   /* ServerPool */
-  luaCtx.registerFunction<void (std::shared_ptr<ServerPool>::*)(std::shared_ptr<DNSDistPacketCache>)>("setCache", [](const std::shared_ptr<ServerPool>& pool, std::shared_ptr<DNSDistPacketCache> cache) {
+  luaCtx.registerFunction<void (std::shared_ptr<dnsdist::lua::LuaServerPoolObject>::*)(std::shared_ptr<DNSDistPacketCache>)>("setCache", [](const std::shared_ptr<dnsdist::lua::LuaServerPoolObject>& pool, std::shared_ptr<DNSDistPacketCache> cache) {
     if (pool) {
-      pool->packetCache = std::move(cache);
+      dnsdist::configuration::updateRuntimeConfiguration([&pool, &cache](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto poolIt = config.d_pools.find(pool->poolName);
+        if (poolIt == config.d_pools.end()) {
+          /* this might happen if the Server Pool has been removed in the meantime, let's gracefully ignore it */
+          return;
+        }
+        poolIt->second.packetCache = std::move(cache);
+      });
+    }
+  });
+  luaCtx.registerFunction<std::shared_ptr<DNSDistPacketCache> (std::shared_ptr<dnsdist::lua::LuaServerPoolObject>::*)() const>("getCache", [](const std::shared_ptr<dnsdist::lua::LuaServerPoolObject>& pool) {
+    std::shared_ptr<DNSDistPacketCache> cache;
+    if (pool) {
+      dnsdist::configuration::updateRuntimeConfiguration([&pool, &cache](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto poolIt = config.d_pools.find(pool->poolName);
+        /* this might happen if the Server Pool has been removed in the meantime, let's gracefully ignore it */
+        if (poolIt != config.d_pools.end()) {
+          cache = poolIt->second.packetCache;
+        }
+      });
+    }
+    return cache;
+  });
+  luaCtx.registerFunction<void (std::shared_ptr<dnsdist::lua::LuaServerPoolObject>::*)()>("unsetCache", [](const std::shared_ptr<dnsdist::lua::LuaServerPoolObject>& pool) {
+    if (pool) {
+      dnsdist::configuration::updateRuntimeConfiguration([&pool](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto poolIt = config.d_pools.find(pool->poolName);
+        if (poolIt == config.d_pools.end()) {
+          /* this might happen if the Server Pool has been removed in the meantime, let's gracefully ignore it */
+          return;
+        }
+        poolIt->second.packetCache.reset();
+      });
     }
   });
-  luaCtx.registerFunction("getCache", &ServerPool::getCache);
-  luaCtx.registerFunction<void (std::shared_ptr<ServerPool>::*)()>("unsetCache", [](const std::shared_ptr<ServerPool>& pool) {
+  luaCtx.registerFunction<bool (std::shared_ptr<dnsdist::lua::LuaServerPoolObject>::*)() const>("getECS", [](const std::shared_ptr<dnsdist::lua::LuaServerPoolObject>& pool) {
+    bool ecs = false;
     if (pool) {
-      pool->packetCache = nullptr;
+      dnsdist::configuration::updateRuntimeConfiguration([&pool, &ecs](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto poolIt = config.d_pools.find(pool->poolName);
+        /* this might happen if the Server Pool has been removed in the meantime, let's gracefully ignore it */
+        if (poolIt != config.d_pools.end()) {
+          ecs = poolIt->second.getECS();
+        }
+      });
+    }
+    return ecs;
+  });
+  luaCtx.registerFunction<void (std::shared_ptr<dnsdist::lua::LuaServerPoolObject>::*)(bool ecs)>("setECS", [](std::shared_ptr<dnsdist::lua::LuaServerPoolObject>& pool, bool ecs) {
+    if (pool) {
+      dnsdist::configuration::updateRuntimeConfiguration([&pool, ecs](dnsdist::configuration::RuntimeConfiguration& config) {
+        auto poolIt = config.d_pools.find(pool->poolName);
+        if (poolIt == config.d_pools.end()) {
+          /* this might happen if the Server Pool has been removed in the meantime, let's gracefully ignore it */
+          return;
+        }
+        poolIt->second.setECS(ecs);
+      });
     }
   });
-  luaCtx.registerFunction("getECS", &ServerPool::getECS);
-  luaCtx.registerFunction("setECS", &ServerPool::setECS);
 
 #ifndef DISABLE_DOWNSTREAM_BINDINGS
   /* DownstreamState */
index 9492361c3450149f8e3a95cbb5f05627ff95966f..dfcf23001da35c1dcb0e49a8936b96d0c9ceb337 100644 (file)
@@ -1260,12 +1260,12 @@ size_t dnsdist_ffi_packetcache_get_domain_list_by_addr(const char* poolName, con
     return 0;
   }
 
-  auto pool = poolIt->second;
-  if (!pool->packetCache) {
+  const auto& pool = poolIt->second;
+  if (!pool.packetCache) {
     return 0;
   }
 
-  auto domains = pool->packetCache->getDomainsContainingRecords(ca);
+  auto domains = pool.packetCache->getDomainsContainingRecords(ca);
   if (domains.size() == 0) {
     return 0;
   }
@@ -1309,12 +1309,12 @@ size_t dnsdist_ffi_packetcache_get_address_list_by_domain(const char* poolName,
     return 0;
   }
 
-  auto pool = poolIt->second;
-  if (!pool->packetCache) {
+  const auto& pool = poolIt->second;
+  if (!pool.packetCache) {
     return 0;
   }
 
-  auto addresses = pool->packetCache->getRecordsForDomain(name);
+  auto addresses = pool.packetCache->getRecordsForDomain(name);
   if (addresses.size() == 0) {
     return 0;
   }
index 151f935647508b0344dba5be6a15f8b250c47bc2..f84fc4af61acedefa6093b9bee0daf9344141132 100644 (file)
@@ -1008,9 +1008,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
   });
 
   luaCtx.writeFunction("getPoolServers", [](const string& pool) {
-    // coverity[auto_causes_copy]
-    const auto poolServers = getDownstreamCandidates(pool);
-    return *poolServers;
+    return getDownstreamCandidates(pool);
   });
 
   luaCtx.writeFunction("getServer", [client](boost::variant<unsigned int, std::string> identifier) -> boost::optional<std::shared_ptr<DownstreamState>> {
@@ -1636,16 +1634,16 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
       const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
       for (const auto& entry : pools) {
         const string& name = entry.first;
-        const std::shared_ptr<ServerPool> pool = entry.second;
-        string cache = pool->packetCache != nullptr ? pool->packetCache->toString() : "";
+        const auto& pool = entry.second;
+        string cache = pool.packetCache != nullptr ? pool.packetCache->toString() : "";
         string policy = defaultPolicyName;
-        if (pool->policy != nullptr) {
-          policy = pool->policy->getName();
+        if (pool.policy != nullptr) {
+          policy = pool.policy->getName();
         }
         string servers;
 
-        const auto poolServers = pool->getServers();
-        for (const auto& server : *poolServers) {
+        const auto& poolServers = pool.getServers();
+        for (const auto& server : poolServers) {
           if (!servers.empty()) {
             servers += ", ";
           }
@@ -1680,10 +1678,19 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
 
   luaCtx.writeFunction("getPool", [client](const string& poolName) {
     if (client) {
-      return std::make_shared<ServerPool>();
+      return std::make_shared<dnsdist::lua::LuaServerPoolObject>(poolName);
     }
-    std::shared_ptr<ServerPool> pool = createPoolIfNotExists(poolName);
-    return pool;
+    bool created = false;
+    dnsdist::configuration::updateRuntimeConfiguration([&poolName, &created](dnsdist::configuration::RuntimeConfiguration& config) {
+      auto [poolIt, inserted] = config.d_pools.emplace(poolName, ServerPool());
+      created = inserted;
+    });
+
+    if (created) {
+      vinfolog("Creating pool %s", poolName);
+    }
+
+    return std::make_shared<dnsdist::lua::LuaServerPoolObject>(poolName);
   });
 
   luaCtx.writeFunction("setVerboseLogDestination", [](const std::string& dest) {
@@ -2069,11 +2076,11 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
   luaCtx.writeFunction("showPoolServerPolicy", [](const std::string& pool) {
     setLuaSideEffect();
     auto poolObj = getPool(pool);
-    if (poolObj->policy == nullptr) {
+    if (poolObj.policy == nullptr) {
       g_outputBuffer = dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName() + "\n";
     }
     else {
-      g_outputBuffer = poolObj->policy->getName() + "\n";
+      g_outputBuffer = poolObj.policy->getName() + "\n";
     }
   });
 #endif /* DISABLE_POLICIES_BINDINGS */
index 73f81329c585f0b4e93b369a9303e4db2ce89d6b..980078c533f99260748cdeecbfdf3ad40fda90b8 100644 (file)
@@ -85,6 +85,16 @@ std::optional<FunctionType> getFunctionFromLuaCode(const std::string& code, cons
 
   return std::nullopt;
 }
+
+struct LuaServerPoolObject
+{
+  LuaServerPoolObject(std::string name) :
+    poolName(std::move(name))
+  {
+  }
+
+  std::string poolName;
+};
 }
 
 namespace dnsdist::configuration::lua
index 48f210001481718f8d7fa4bcd979bc01d5677712..36bc0519f452e3bad20b39507e2d9e0898143694 100644 (file)
@@ -1178,7 +1178,7 @@ public:
   bool matches(const DNSQuestion* dnsQuestion) const override
   {
     (void)dnsQuestion;
-    return (getPool(d_poolname)->countServers(true) > 0);
+    return (getPool(d_poolname).countServers(true) > 0);
   }
 
   string toString() const override
@@ -1201,7 +1201,7 @@ public:
   bool matches(const DNSQuestion* dnsQuestion) const override
   {
     (void)dnsQuestion;
-    return (getPool(d_poolname)->poolLoad()) > d_limit;
+    return (getPool(d_poolname).poolLoad()) > d_limit;
   }
 
   string toString() const override
diff --git a/pdns/dnsdistdist/dnsdist-server-pool.hh b/pdns/dnsdistdist/dnsdist-server-pool.hh
new file mode 100644 (file)
index 0000000..84e48f1
--- /dev/null
@@ -0,0 +1,63 @@
+/*
+ * This file is part of PowerDNS or dnsdist.
+ * Copyright -- PowerDNS.COM B.V. and its contributors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * In addition, for the avoidance of any doubt, permission is granted to
+ * link this program with OpenSSL and to (re)distribute the binaries
+ * produced as the result of such linking.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#pragma once
+#include <memory>
+
+#include "dnsdist-lbpolicies.hh"
+
+class DNSDistPacketCache;
+
+struct ServerPool
+{
+  const std::shared_ptr<DNSDistPacketCache> getCache() const
+  {
+    return packetCache;
+  }
+
+  bool getECS() const
+  {
+    return d_useECS;
+  }
+
+  void setECS(bool useECS)
+  {
+    d_useECS = useECS;
+  }
+
+  size_t poolLoad() const;
+  size_t countServers(bool upOnly) const;
+  const ServerPolicy::NumberedServerVector& getServers() const;
+  void addServer(std::shared_ptr<DownstreamState>& server);
+  void removeServer(std::shared_ptr<DownstreamState>& server);
+  bool isTCPOnly() const
+  {
+    return d_tcpOnly;
+  }
+
+  std::shared_ptr<DNSDistPacketCache> packetCache{nullptr};
+  std::shared_ptr<ServerPolicy> policy{nullptr};
+
+private:
+  ServerPolicy::NumberedServerVector d_servers;
+  bool d_useECS{false};
+  bool d_tcpOnly{false};
+};
index d7a91e60b4deb14f7364acba2facfe204a005eaf..3103742a3903e1c9f22c88f38ac542fe31e7ecc9 100644 (file)
@@ -867,12 +867,12 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp)
       poolName = "_default_";
     }
     const string label = "{pool=\"" + poolName + "\"}";
-    const std::shared_ptr<ServerPool> pool = entry.second;
-    output << "dnsdist_pool_servers" << label << " " << pool->countServers(false) << "\n";
-    output << "dnsdist_pool_active_servers" << label << " " << pool->countServers(true) << "\n";
+    const auto& pool = entry.second;
+    output << "dnsdist_pool_servers" << label << " " << pool.countServers(false) << "\n";
+    output << "dnsdist_pool_active_servers" << label << " " << pool.countServers(true) << "\n";
 
-    if (pool->packetCache != nullptr) {
-      const auto& cache = pool->packetCache;
+    if (pool.packetCache != nullptr) {
+      const auto& cache = pool.packetCache;
 
       output << cachebase << "cache_size"              <<label << " " << cache->getMaxEntries()       << "\n";
       output << cachebase << "cache_entries"           <<label << " " << cache->getEntriesCount()     << "\n";
@@ -1251,11 +1251,11 @@ static void handleStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
     const auto& localPools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
     pools.reserve(localPools.size());
     for (const auto& pool : localPools) {
-      const auto& cache = pool.second->packetCache;
+      const auto& cache = pool.second.packetCache;
       Json::object entry{
         {"id", num++},
         {"name", pool.first},
-        {"serversCount", (double)pool.second->countServers(false)},
+        {"serversCount", (double)pool.second.countServers(false)},
         {"cacheSize", (double)(cache ? cache->getMaxEntries() : 0)},
         {"cacheEntries", (double)(cache ? cache->getEntriesCount() : 0)},
         {"cacheHits", (double)(cache ? cache->getHits() : 0)},
@@ -1362,10 +1362,10 @@ static void handlePoolStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
   }
 
   const auto& pool = poolIt->second;
-  const auto& cache = pool->packetCache;
+  const auto& cache = pool.packetCache;
   Json::object entry{
     {"name", poolName->second},
-    {"serversCount", (double)pool->countServers(false)},
+    {"serversCount", (double)pool.countServers(false)},
     {"cacheSize", (double)(cache ? cache->getMaxEntries() : 0)},
     {"cacheEntries", (double)(cache ? cache->getEntriesCount() : 0)},
     {"cacheHits", (double)(cache ? cache->getHits() : 0)},
@@ -1379,7 +1379,7 @@ static void handlePoolStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
 
   Json::array servers;
   int num = 0;
-  for (const auto& server : *pool->getServers()) {
+  for (const auto& server : pool.getServers()) {
     addServerToJSON(servers, num, server.second);
     num++;
   }
@@ -1585,7 +1585,7 @@ static void handleCacheManagement(const YaHTTP::Request& req, YaHTTP::Response&
     type = QType::chartocode(expungeType->second.c_str());
   }
 
-  std::shared_ptr<ServerPool> pool;
+  std::optional<ServerPool> pool;
   try {
     pool = getPool(poolName->second);
   }
index 520bcc57398edaf1449ddfc6352316bb03a02344..56bd04fb8206fdcb2c7e8957a5e1c5781f9deeb8 100644 (file)
@@ -1424,12 +1424,11 @@ static ProcessQueryResult handleQueryTurnedIntoSelfAnsweredResponse(DNSQuestion&
   return ProcessQueryResult::SendAnswer;
 }
 
-static void selectBackendForOutgoingQuery(DNSQuestion& dnsQuestion, const std::shared_ptr<ServerPool>& serverPool, std::shared_ptr<DownstreamState>& selectedBackend)
+static void selectBackendForOutgoingQuery(DNSQuestion& dnsQuestion, const ServerPool& serverPool, std::shared_ptr<DownstreamState>& selectedBackend)
 {
-  std::shared_ptr<ServerPolicy> poolPolicy = serverPool->policy;
-  const auto& policy = poolPolicy != nullptr ? *poolPolicy : *dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy;
-  const auto servers = serverPool->getServers();
-  selectedBackend = policy.getSelectedBackend(*servers, dnsQuestion);
+  const auto& policy = serverPool.policy != nullptr ? *serverPool.policy : *dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy;
+  const auto& servers = serverPool.getServers();
+  selectedBackend = policy.getSelectedBackend(servers, dnsQuestion);
 }
 
 ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ptr<DownstreamState>& selectedBackend)
@@ -1440,15 +1439,15 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_
     if (dnsQuestion.getHeader()->qr) { // something turned it into a response
       return handleQueryTurnedIntoSelfAnsweredResponse(dnsQuestion);
     }
-    std::shared_ptr<ServerPool> serverPool = getPool(dnsQuestion.ids.poolName);
-    dnsQuestion.ids.packetCache = serverPool->packetCache;
+    const auto& serverPool = getPool(dnsQuestion.ids.poolName);
+    dnsQuestion.ids.packetCache = serverPool.packetCache;
     selectBackendForOutgoingQuery(dnsQuestion, serverPool, selectedBackend);
     bool willBeForwardedOverUDP = !dnsQuestion.overTCP() || dnsQuestion.ids.protocol == dnsdist::Protocol::DoH;
     if (selectedBackend && selectedBackend->isTCPOnly()) {
       willBeForwardedOverUDP = false;
     }
     else if (!selectedBackend) {
-      willBeForwardedOverUDP = !serverPool->isTCPOnly();
+      willBeForwardedOverUDP = !serverPool.isTCPOnly();
     }
 
     uint32_t allowExpired = selectedBackend ? 0 : dnsdist::configuration::getCurrentRuntimeConfiguration().d_staleCacheEntriesTTL;
@@ -1457,7 +1456,7 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_
       dnsQuestion.ids.dnssecOK = (dnsdist::getEDNSZ(dnsQuestion) & EDNS_HEADER_FLAG_DO) != 0;
     }
 
-    if (dnsQuestion.useECS && ((selectedBackend && selectedBackend->d_config.useECS) || (!selectedBackend && serverPool->getECS()))) {
+    if (dnsQuestion.useECS && ((selectedBackend && selectedBackend->d_config.useECS) || (!selectedBackend && serverPool.getECS()))) {
       // we special case our cache in case a downstream explicitly gave us a universally valid response with a 0 scope
       // we need ECS parsing (parseECS) to be true so we can be sure that the initial incoming query did not have an existing
       // ECS option, which would make it unsuitable for the zero-scope feature.
@@ -1541,9 +1540,9 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_
       /* let's be nice and allow the selection of a different pool,
          but no second cache-lookup for you */
       if (dnsQuestion.ids.poolName != existingPool) {
-        serverPool = getPool(dnsQuestion.ids.poolName);
-        dnsQuestion.ids.packetCache = serverPool->packetCache;
-        selectBackendForOutgoingQuery(dnsQuestion, serverPool, selectedBackend);
+        const auto& newServerPool = getPool(dnsQuestion.ids.poolName);
+        dnsQuestion.ids.packetCache = newServerPool.packetCache;
+        selectBackendForOutgoingQuery(dnsQuestion, newServerPool, selectedBackend);
       }
     }
 
@@ -2341,7 +2340,7 @@ static void maintThread()
       for (const auto& entry : pools) {
         const auto& pool = entry.second;
 
-        auto packetCache = pool->packetCache;
+        const auto& packetCache = pool.packetCache;
         if (!packetCache) {
           continue;
         }
@@ -2353,7 +2352,7 @@ static void maintThread()
            has all its backends down) */
         if (packetCache->keepStaleData() && !iter->second) {
           /* so far all pools had at least one backend up */
-          if (pool->countServers(true) == 0) {
+          if (pool.countServers(true) == 0) {
             iter->second = true;
           }
         }
@@ -3147,7 +3146,7 @@ static void setupPools()
   }
   else {
     for (const auto& entry : currentConfig.d_pools) {
-      if (entry.second->policy != nullptr && entry.second->policy->getName() == "chashed") {
+      if (entry.second.policy != nullptr && entry.second.policy->getName() == "chashed") {
         precompute = true;
         break;
       }
index 765dce17d56bef6f5ac6f8a4521a81cccb22fe1d..4dc8dfaefca3d2c543aa876909aa8f456bc279f5 100644 (file)
@@ -929,51 +929,6 @@ public:
 
 void responderThread(std::shared_ptr<DownstreamState> dss);
 
-class DNSDistPacketCache;
-
-struct ServerPool
-{
-  ServerPool() :
-    d_servers(std::make_shared<const ServerPolicy::NumberedServerVector>())
-  {
-  }
-
-  ~ServerPool()
-  {
-  }
-
-  const std::shared_ptr<DNSDistPacketCache> getCache() const { return packetCache; };
-
-  bool getECS() const
-  {
-    return d_useECS;
-  }
-
-  void setECS(bool useECS)
-  {
-    d_useECS = useECS;
-  }
-
-  std::shared_ptr<DNSDistPacketCache> packetCache{nullptr};
-  std::shared_ptr<ServerPolicy> policy{nullptr};
-
-  size_t poolLoad();
-  size_t countServers(bool upOnly);
-  const std::shared_ptr<const ServerPolicy::NumberedServerVector> getServers();
-  void addServer(shared_ptr<DownstreamState>& server);
-  void removeServer(shared_ptr<DownstreamState>& server);
-  bool isTCPOnly() const
-  {
-    // coverity[missing_lock]
-    return d_tcpOnly;
-  }
-
-private:
-  SharedLockGuarded<std::shared_ptr<const ServerPolicy::NumberedServerVector>> d_servers;
-  bool d_useECS{false};
-  bool d_tcpOnly{false};
-};
-
 enum ednsHeaderFlags
 {
   EDNS_HEADER_FLAG_NONE = 0,
index 0d7cee13c3943ac1ec330cf71ba984d522f67512..3525f803c071fb7f6eeeab75a1fbeb450c1eebea 100644 (file)
@@ -505,10 +505,10 @@ BOOST_AUTO_TEST_CASE(test_PacketCache)
   packetCache->insert(key, subnet, *(getFlagsFromDNSHeader(dq.getHeader().get())), dnssecOK, ids.qname, QType::A, QClass::IN, response, receivedOverUDP, 0, boost::none);
 
   std::string poolName("test-pool");
-  auto testPool = std::make_shared<ServerPool>();
-  testPool->packetCache = packetCache;
+  auto testPool = ServerPool();
+  testPool.packetCache = packetCache;
   std::string poolWithNoCacheName("test-pool-without-cache");
-  auto testPoolWithNoCache = std::make_shared<ServerPool>();
+  auto testPoolWithNoCache = ServerPool();
   dnsdist::configuration::updateRuntimeConfiguration([&poolName, &testPool, &poolWithNoCacheName, &testPoolWithNoCache](dnsdist::configuration::RuntimeConfiguration& config) {
     config.d_pools.emplace(poolName, testPool);
     config.d_pools.emplace(poolWithNoCacheName, testPoolWithNoCache);