[stats] lazyinit ClusterInfo::trafficStats() #23921
[stats] lazyinit ClusterInfo::trafficStats() #23921jmarantz merged 145 commits intoenvoyproxy:mainfrom
Conversation
…n non-config-update stats Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
…-stats and "the rest"(will be renamed to upstream-stats) Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
/assign @jmarantz |
jmarantz
left a comment
There was a problem hiding this comment.
generally looks good but why don't we get the other PR in first.
|
Hey -- I converted this to a draft until #23907 lands. Feel free to convert back when that lands and you merge main into this. |
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
please take a look at the last column in the updated table, in PR description. |
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
/wait Once the factored-out infrastructure in #27899 merges, this PR can merge main and will become smaller and easier to polish off with a final check. |
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
envoy/upstream/upstream.h
Outdated
| #include "fmt/format.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
There was a problem hiding this comment.
nit: remove empty line.
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
/retest |
jmarantz
left a comment
There was a problem hiding this comment.
One minor nit, but basically looks great. This has come a long way in readability.
@ggreenway can you take a pass?
| ClusterInfoImpl::generateStats(Stats::ScopeSharedPtr scope, | ||
| const ClusterTrafficStatNames& stat_names, bool deferred_creation) { | ||
| return Stats::createDeferredCompatibleStats<ClusterTrafficStats>(scope, stat_names, | ||
| deferred_creation); |
There was a problem hiding this comment.
nit: s/deferred_creation/defer_creation/ as this applies to something that will happen when the function is called, not something that already happened.
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
| ClusterInfoImpl::generateStats(Stats::ScopeSharedPtr scope, | ||
| const ClusterTrafficStatNames& stat_names, bool deferred_creation) { | ||
| return Stats::createDeferredCompatibleStats<ClusterTrafficStats>(scope, stat_names, | ||
| deferred_creation); |
envoy/upstream/upstream.h
Outdated
| #include "fmt/format.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
|
/retest |
1 similar comment
|
/retest |
|
@ggreenway are you able to take a look soon? I think this is much easier to digest now . |
ggreenway
left a comment
There was a problem hiding this comment.
LGTM except it should have a changelog entry
/wait
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
Thanks. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
Commit Message: LazyInit ClusterInfo::trafficStats().
Additional Description:
3rd PR for #23575, this is stack upon 2nd PR #23907
With 100K clusters, we are seeing ~1.5 GBs less RAM usage with this PR.
Kudos to @jmarantz for the data:
To reproduce, use this script to add 100K clusters to the bootstrap tmpl:
base.tmpl:
Run the following commands to generate the bootstrap.
and modify the test/config/main_common_test.cc to load from test/config/integration/100k_clusters.yaml instead. Then you could run the test and observe the RAM uasage.
Risk Level: Medium (changes how cluster_info::trafficStats() are created).
Testing: existing unit tests.
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]