-
Notifications
You must be signed in to change notification settings - Fork 886
fix(sei-tendermint): bound P2P and consensus Prometheus label cardinality (PLT-1070, PLT-1071) #4029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
fix(sei-tendermint): bound P2P and consensus Prometheus label cardinality (PLT-1070, PLT-1071) #4029
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
e8dc19c
fix(sei-tendermint): bound P2P and consensus Prometheus label cardina…
amir-deris 4c9cf93
fix(sei-tendermint): gate late_votes labels to the active validator set
amir-deris 8755d2c
Merge branch 'main' into amir/plt-1070-plt-1071-metric-cardinality-issue
masih File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] Active-set gating bounds the rate of new series but not the total: a Prometheus
CounterVecnever evicts a child, so every address that was in the set at the moment it sent a late vote keeps its series for the life of the process. The cap is therefore|union of validator sets over process lifetime| + 1, not the~|validator set| + 1the PR description states.That is still a large improvement — the unbounded axis (peer churn, and pre-fix any peer minting a series with an arbitrary
ValidatorAddress, since this marking happens before the vote's validator is checked against the set) is gone, and validator churn is slow and governance-paced. So this is worth adjusting the claim in the description/help text for rather than redesigning the label. If a hard cap is actually required, it needs an explicitDeleteLabelValuessweep on validator-set change, which is a meaningfully larger change and probably not warranted here.