stats: remove deprecated ScopeStatsLimitSettings#44211
Closed
ebm wants to merge 1 commit intoenvoyproxy:mainfrom
Closed
stats: remove deprecated ScopeStatsLimitSettings#44211ebm wants to merge 1 commit intoenvoyproxy:mainfrom
ebm wants to merge 1 commit intoenvoyproxy:mainfrom
Conversation
ScopeStatsLimitSettings limited the number of stats each scope could create. This was necessary when stats lived in shared memory, to prevent one scope from hogging the shared space. Stats now allocate from the heap, so these limits are unnecessary. ScopeStatsLimitSettings has been deprecated since then, leaving the struct as dead code. Signed-off-by: Ethan Marantz <ebmarantz@gmail.com>
ebm
requested a deployment
to
external-contributors
GitHub Actions
Waiting
Contributor
|
Note: this is a fix for #44162 |
jmarantz
reviewed
Apr 1, 2026
Contributor
jmarantz
left a comment
jmarantz
left a comment
There was a problem hiding this comment.
this looks close to perfect assuming it passes CI.
| static_assert(std::is_same_v<StatType, TextReadout>, "Unexpected StatType"); | ||
| } | ||
| // Limits don't need to be checked when creating stats because each scope's stats do not | ||
| // live in shared memory. |
Contributor
There was a problem hiding this comment.
I think we can just drop this comment
ggreenway
requested changes
Apr 1, 2026
Member
ggreenway
left a comment
ggreenway
left a comment
There was a problem hiding this comment.
I think this code was just added in January, and is going to be used in #43625
cc @TAOXUY and @wbpcode and @kyessenov
Contributor
|
Yeah, this is a work-in-progress. Is there an issue that this is solving? |
Contributor
|
Oh this is my bad. I thought this was a leftover remnant of the old shared-memory-limited stats. I didn't realize it was for future use. @ebm let's find a different thing but at least you have the process sorted out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: stats: remove deprecated ScopeStatsLimitSettings
Additional Description: ScopeStatsLimitSettings limited the number of stats each scope could create. This was necessary when stats lived in shared memory, to prevent one scope from hogging the shared space. Stats now allocate from the heap, so these limits are unnecessary. ScopeStatsLimitSettings has been deprecated since then, leaving the struct as dead code.
Risk Level: Low
Testing: Removed tests that validated limit stats behavior. Changed tests that referenced ScopeStatsLimitSettings. All existing tests pass.
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A