envoyproxy / envoyproxy/envoy

Reduce used cluster memory by initialising http_async_client_ lazily

Open
#19,332 13 comments 0 reactions 0 assignees View on GitHub
area/perf help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

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(
+ 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;
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.