Skip to content
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

Ignore seen aggregates #3439

Merged
merged 2 commits into from
Feb 25, 2022
Merged

Ignore seen aggregates #3439

merged 2 commits into from
Feb 25, 2022

Conversation

arnetheduck
Copy link
Member

ethereum/consensus-specs#2225 removed an ignore
rule that would filter out duplicate aggregates from gossip publishing -
however, this causes increased bandwidth and CPU usage as discussed in
ethereum/consensus-specs#2183 - the intent is
to revert the removal and reinstate the rule.

This PR implements ignore filtering which cuts down on CPU usage (fewer
aggregates to validate) and bandwidth usage (less fanout of duplicates)

ethereum/consensus-specs#2225 removed an ignore
rule that would filter out duplicate aggregates from gossip publishing -
however, this causes increased bandwidth and CPU usage as discussed in
ethereum/consensus-specs#2183 - the intent is
to revert the removal and reinstate the rule.

This PR implements ignore filtering which cuts down on CPU usage (fewer
aggregates to validate) and bandwidth usage (less fanout of duplicates)
- as #2225 points out, this may lead to a small increase in IHAVE
messages.
@arnetheduck arnetheduck enabled auto-merge (squash) February 25, 2022 15:05
@arnetheduck arnetheduck merged commit 92e7e28 into unstable Feb 25, 2022
@arnetheduck arnetheduck deleted the ignore-dupe-agg branch February 25, 2022 16:15
@github-actions
Copy link

Unit Test Results

     12 files  ±0     821 suites  ±0   32m 42s ⏱️ +12s
1 671 tests ±0  1 625 ✔️ ±0    46 💤 ±0  0 ±0 
9 755 runs  ±0  9 655 ✔️ ±0  100 💤 ±0  0 ±0 

Results for commit bde8728. ± Comparison against base commit 1bfbcc4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants