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

feat: remove peers after several unsuccessful attempts #3780

Merged
merged 2 commits into from
Jul 14, 2023

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jul 14, 2023

supersedes #3076

we're tracking what kind of error caused a failed dial/disconnect. if the error is severe, for example connection refused we backoff in and increase a counter.

previously peers, even offline peers weren't cleared from the peerset, but backed off with increasing duration.

this adds a new config threshold after which peers are removed from the set, for example if a peer is not reachable after the 5th attempt, it is removed from the set

@mattsse mattsse requested a review from Rjected as a code owner July 14, 2023 11:18
@mattsse mattsse added C-perf A change motivated by improving speed, memory usage or disk footprint A-networking Related to networking in general labels Jul 14, 2023
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #3780 (10c28bd) into main (e3ac77a) will increase coverage by 8.32%.
The diff coverage is 85.07%.

Impacted file tree graph

Impacted Files Coverage Δ
crates/net/network/src/error.rs 76.04% <50.00%> (+16.04%) ⬆️
crates/net/network/src/peers/manager.rs 83.82% <85.93%> (+8.13%) ⬆️
crates/config/src/config.rs 64.65% <100.00%> (ø)

... and 189 files with indirect coverage changes

Flag Coverage Δ
integration-tests 12.73% <14.92%> (-3.06%) ⬇️
unit-tests 64.31% <85.07%> (+10.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 26.15% <100.00%> (+1.41%) ⬆️
blockchain tree 83.02% <ø> (+41.29%) ⬆️
pipeline 87.13% <ø> (+7.89%) ⬆️
storage (db) 73.27% <ø> (+14.10%) ⬆️
trie 94.65% <ø> (+23.06%) ⬆️
txpool 48.76% <ø> (+4.69%) ⬆️
networking 77.33% <84.84%> (+8.73%) ⬆️
rpc 49.94% <ø> (-1.47%) ⬇️
consensus 64.90% <ø> (+8.10%) ⬆️
revm 32.94% <ø> (+4.26%) ⬆️
payload builder 4.13% <ø> (-2.49%) ⬇️
primitives 87.99% <ø> (+9.66%) ⬆️

@mattsse mattsse requested a review from gakonst as a code owner July 14, 2023 11:45
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems reasonable for removing offline or otherwise misbehaving peers from the peerset, lgtm

@mattsse mattsse added this pull request to the merge queue Jul 14, 2023
Merged via the queue into main with commit 77faa04 Jul 14, 2023
@mattsse mattsse deleted the matt/remove-peers-after-several-severe-backoffs branch July 14, 2023 19:23
merklefruit pushed a commit to anton-rs/op-reth that referenced this pull request Jul 18, 2023
merklefruit pushed a commit to merklefruit/op-reth-old that referenced this pull request Jul 26, 2023
merklefruit pushed a commit to anton-rs/op-reth that referenced this pull request Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-perf A change motivated by improving speed, memory usage or disk footprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants