Reduce load on validator subscription channels#5311
Merged
mergify[bot] merged 4 commits intosigp:unstablefrom Mar 7, 2024
Merged
Reduce load on validator subscription channels#5311mergify[bot] merged 4 commits intosigp:unstablefrom
mergify[bot] merged 4 commits intosigp:unstablefrom
Conversation
AgeManning
approved these changes
Feb 27, 2024
Member
AgeManning
left a comment
AgeManning
left a comment
There was a problem hiding this comment.
Yeah nice. Reducing messages ftw
pawanjay176
added
the
optimization
Member
Author
|
@michaelsproul Can we roll this in with #5305 ? |
michaelsproul
approved these changes
Mar 7, 2024
Member
michaelsproul
left a comment
michaelsproul
left a comment
There was a problem hiding this comment.
Looks good with the BTreeSet tweak!
Happy to merge and continue testing on infra over the weekend
pawanjay176
added
ready-for-merge
Member
Author
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at 84a902a |
This was referenced Mar 7, 2024
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.
Issue Addressed
N/A
Proposed Changes
Since we now only subscribe to a fixed number of long lived subnets per node, the
validator_indexfield inValidatorSubscriptiontype is entirely inconsequential to the beacon node.This PR does 2 things:
validator_indexfield from theValidatorSubscriptiontype. This allows us to filter duplicates which can be quite a lot in high validator count setups.I'm not completely sure at what size of the vector we'll start seeing slowness in the channel, but by filtering duplicates, we shouldn't be sending too many.
Max length of the vector would be
max_committees_per_slot * slots_per_epoch * 2(aggregator bit) = 4096.Not super attached to this change, but if we send one by one, then we should probably change the
ValidatorSubscriptionMessageto take just a single message instead of a vector for consistency.