envoyproxy/envoy

Reduce used cluster memory by initialising http_async_client_ lazily

Open

#19,332 opened on Dec 21, 2021

View on GitHub
 (13 comments) (0 reactions) (0 assignees)C++ (5,373 forks)batch import
area/perfhelp wanted

Repository metrics

Stars
 (27,997 stars)
PR merge metrics
 (Avg merge 8d) (378 merged PRs in 30d)

Description

Title: Reduce used cluster memory by initialising http_async_client_ lazily

Description: I've been trying to put my envoy on a diet and noticed that most (if not all) of my clusters do not seemingly use httpAsyncClient() but I am paying a price for it in terms of used memory. I have 2500 TLS clusters and by default I end up with about ~249.7m used memory in idle state. If I were to make http_async_client_ lazy given the below patch I end up with ~209.7m used memory which is a huge difference. Below is the patch I used for measuring though I haven't tested it yet if it works for clusters that actually need the httpAsyncClient().

I'll look around and see what else can be initialised lazily.

diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
index 703c1b7..791b917 100644
--- a/source/common/upstream/cluster_manager_impl.cc
+++ b/source/common/upstream/cluster_manager_impl.cc
@@ -1048,7 +1048,16 @@ Host::CreateConnectionData ClusterManagerImpl::ThreadLocalClusterManagerImpl::Cl

 Http::AsyncClient&
 ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::httpAsyncClient() {
-  return http_async_client_;
+  if (!http_async_client_) {
+      http_async_client_ =
+          std::make_unique<Http::AsyncClientImpl>(
+              cluster_info_, parent_.parent_.stats_, parent_.thread_local_dispatcher_,
+              parent_.parent_.local_info_, parent_.parent_, parent_.parent_.runtime_,
+              parent_.parent_.random_,
+              Router::ShadowWriterPtr{new Router::ShadowWriterImpl(parent_.parent_)},
+              parent_.parent_.http_context_, parent_.parent_.router_context_);
+  }
+  return *http_async_client_;
 }

 ClusterUpdateCallbacksHandlePtr
@@ -1346,12 +1355,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::getHttpConnPoolsContainer(
 ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry(
     ThreadLocalClusterManagerImpl& parent, ClusterInfoConstSharedPtr cluster,
     const LoadBalancerFactorySharedPtr& lb_factory)
-    : parent_(parent), lb_factory_(lb_factory), cluster_info_(cluster),
-      http_async_client_(cluster, parent.parent_.stats_, parent.thread_local_dispatcher_,
-                         parent.parent_.local_info_, parent.parent_, parent.parent_.runtime_,
-                         parent.parent_.random_,
-                         Router::ShadowWriterPtr{new Router::ShadowWriterImpl(parent.parent_)},
-                         parent_.parent_.http_context_, parent_.parent_.router_context_) {
+    : parent_(parent), lb_factory_(lb_factory), cluster_info_(cluster) {
   priority_set_.getOrCreateHostSet(0);

   // TODO(mattklein123): Consider converting other LBs over to thread local. All of them could
diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h
index 2107ec2..0ff4fdd 100644
--- a/source/common/upstream/cluster_manager_impl.h
+++ b/source/common/upstream/cluster_manager_impl.h
@@ -431,7 +431,7 @@ private:
       // Current active LB.
       LoadBalancerPtr lb_;
       ClusterInfoConstSharedPtr cluster_info_;
-      Http::AsyncClientImpl http_async_client_;
+      Http::AsyncClientPtr http_async_client_;
     };

     using ClusterEntryPtr = std::unique_ptr<ClusterEntry>;

Contributor guide