dnsdist: Limit # of proxy protocol-enabled outgoing TCP connections

TCP worker threads keep a cache of outgoing TCP connections to a
backend to be able to reuse them for subsequent queries. Proxy
protocol-enabled outgoing TCP connections are trickier because the
proxy protocol payload is sent only once at the beginning of the
TCP connection, contains the source and destination addresses and
ports, and thus the connections can only be reused with the exact
same incoming TCP connection. For this reason these connections are
stored in a specific structure of the incoming connection, instead
of the TCP worker connection cache. However, we can only reuse a
given proxy protocol-enabled outgoing TCP connection for a subsequent
query if the TLV values contained in the proxy-protocol payload
associated to the new query are exactly the same than the ones
associated to the existing query. Up until now, we would keep an
unbounded amount of proxy protocol-enabled connections around if
the TLV values were, for example, randomly assigned per query.
This commit sets a limit on the number of such connections we will
keep around: we will keep at most N connections, where N is the
ratio between the number of concurrent queries on a single TCP
connection supported by the backend and the number of concurrent
queries on a single TCP connection supported by the frontend, with
a hard cap to 5.

diff --git a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
index e7c3a1a3b9246120f6e7688b7debd2a577daf7ad..4e66bd58bffcf0fc3abb4d078f894ce622803968 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
+++ b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
@@ -115,7 +115,6 @@ public:
     return false;
   }
 
-  std::shared_ptr<TCPConnectionToBackend> getOwnedDownstreamConnection(const std::shared_ptr<DownstreamState>& backend, const std::unique_ptr<std::vector<ProxyProtocolValue>>& tlvs);
   std::shared_ptr<TCPConnectionToBackend> getDownstreamConnection(std::shared_ptr<DownstreamState>& backend, const std::unique_ptr<std::vector<ProxyProtocolValue>>& tlvs, const struct timeval& now);
   void registerOwnedDownstreamConnection(std::shared_ptr<TCPConnectionToBackend>& conn);
 

diff --git a/pdns/dnsdistdist/dnsdist-tcp.cc b/pdns/dnsdistdist/dnsdist-tcp.cc
index 5454a156a8eccaa06d1e27dbf643958c89cc55af..db61d8817fdf37396a9fc81c74304378f3a94c59 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp.cc
+++ b/pdns/dnsdistdist/dnsdist-tcp.cc
@@ -99,14 +99,41 @@ size_t IncomingTCPConnectionState::clearAllDownstreamConnections()
   return t_downstreamTCPConnectionsManager.clear();
 }
 
+static std::pair<std::shared_ptr<TCPConnectionToBackend>, bool> getOwnedDownstreamConnection(std::map<std::shared_ptr<DownstreamState>, std::deque<std::shared_ptr<TCPConnectionToBackend>>>& ownedConnectionsToBackend, const std::shared_ptr<DownstreamState>& backend, const std::unique_ptr<std::vector<ProxyProtocolValue>>& tlvs)
+{
+  bool tlvsMismatch = false;
+  auto connIt = ownedConnectionsToBackend.find(backend);
+  if (connIt == ownedConnectionsToBackend.end()) {
+    DEBUGLOG("no owned connection found for " << backend->getName());
+    return {nullptr, tlvsMismatch};
+  }
+
+  for (auto& conn : connIt->second) {
+    if (conn->canBeReused(true)) {
+      if (conn->matchesTLVs(tlvs)) {
+        DEBUGLOG("Got one owned connection accepting more for " << backend->getName());
+        conn->setReused();
+        return {conn, tlvsMismatch};
+      }
+      DEBUGLOG("Found one connection to " << backend->getName() << " but with different TLV values");
+      tlvsMismatch = true;
+    }
+    DEBUGLOG("not accepting more for " << backend->getName());
+  }
+
+  return {nullptr, tlvsMismatch};
+}
+
 std::shared_ptr<TCPConnectionToBackend> IncomingTCPConnectionState::getDownstreamConnection(std::shared_ptr<DownstreamState>& backend, const std::unique_ptr<std::vector<ProxyProtocolValue>>& tlvs, const struct timeval& now)
 {
-  auto downstream = getOwnedDownstreamConnection(backend, tlvs);
+  auto [downstream, tlvsMismatch] = getOwnedDownstreamConnection(d_ownedConnectionsToBackend, backend, tlvs);
 
   if (!downstream) {
     /* we don't have a connection to this backend owned yet, let's get one (it might not be a fresh one, though) */
     downstream = t_downstreamTCPConnectionsManager.getConnectionToDownstream(d_threadData.mplexer, backend, now, std::string());
-    if (backend->d_config.useProxyProtocol) {
+    // if we had an existing connection but the TLVs are different, they are likely unique per query so do not bother keeping the connection
+    // around
+    if (backend->d_config.useProxyProtocol && !tlvsMismatch) {
       registerOwnedDownstreamConnection(downstream);
     }
   }
@@ -260,29 +287,26 @@ void IncomingTCPConnectionState::resetForNewQuery()
   d_state = State::waitingForQuery;
 }
 
-std::shared_ptr<TCPConnectionToBackend> IncomingTCPConnectionState::getOwnedDownstreamConnection(const std::shared_ptr<DownstreamState>& backend, const std::unique_ptr<std::vector<ProxyProtocolValue>>& tlvs)
-{
-  auto connIt = d_ownedConnectionsToBackend.find(backend);
-  if (connIt == d_ownedConnectionsToBackend.end()) {
-    DEBUGLOG("no owned connection found for " << backend->getName());
-    return nullptr;
-  }
-
-  for (auto& conn : connIt->second) {
-    if (conn->canBeReused(true) && conn->matchesTLVs(tlvs)) {
-      DEBUGLOG("Got one owned connection accepting more for " << backend->getName());
-      conn->setReused();
-      return conn;
-    }
-    DEBUGLOG("not accepting more for " << backend->getName());
-  }
-
-  return nullptr;
-}
-
 void IncomingTCPConnectionState::registerOwnedDownstreamConnection(std::shared_ptr<TCPConnectionToBackend>& conn)
 {
-  d_ownedConnectionsToBackend[conn->getDS()].push_front(conn);
+  const auto& downstream = conn->getDS();
+  auto& queue = d_ownedConnectionsToBackend[downstream];
+  // how many proxy-protocol enabled connections do we want to keep around?
+  // - they are only usable for this incoming connection because of the proxy protocol header containing the source and destination addresses and ports
+  // - if we have TLV values, and they are unique per query, keeping these is useless
+  // - if there is no, or identical, TLV values, a single outgoing connection is enough if maxInFlight == 1, or if incoming maxInFlight == outgoing maxInFlight
+  // so it makes sense to keep a few of them around if incoming maxInFlight is greater than outgoing maxInFlight
+
+  auto incomingMaxInFlightQueriesPerConn = d_ci.cs->d_maxInFlightQueriesPerConn;
+  incomingMaxInFlightQueriesPerConn = std::max(incomingMaxInFlightQueriesPerConn, static_cast<size_t>(1U));
+  auto outgoingMaxInFlightQueriesPerConn = downstream->d_config.d_maxInFlightQueriesPerConn;
+  outgoingMaxInFlightQueriesPerConn = std::max(outgoingMaxInFlightQueriesPerConn, static_cast<size_t>(1U));
+  size_t maxCachedOutgoingConnections = std::min(static_cast<size_t>(std::round(incomingMaxInFlightQueriesPerConn / outgoingMaxInFlightQueriesPerConn)), static_cast<size_t>(5U));
+
+  queue.push_front(conn);
+  if (queue.size() > maxCachedOutgoingConnections) {
+    queue.pop_back();
+  }
 }
 
 /* called when the buffer has been set and the rules have been processed, and only from handleIO (sometimes indirectly via handleQuery) */