Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Make peer evictions less aggressive#14619

Merged
bkchr merged 4 commits intomasterfrom
sync-adjust-eviction-threshold
Aug 15, 2023
Merged

Make peer evictions less aggressive#14619
bkchr merged 4 commits intomasterfrom
sync-adjust-eviction-threshold

Conversation

@altonen
Copy link
Contributor

@altonen altonen commented Jul 24, 2023

The original implementation of peer eviction prioritized aliveness over connection stability which made the peer count unstable for some users.

As this may cause discomfort or infrastructure alerts if stability is tracked, adjust the eviction to be less aggressive by only evicting peers when the node has fully stalled. This causes the node to have some peers who are inactive and won't send any block announcements. These nodes are removed if the local node is able to receive at least one block announcement from one of its peers as the inactivity of the substream is detected when a notification is sent.

If the node won't send or receive any block annoucements for 30 seconds, it's considered stalled and it will evict all peers, causing ProtocolController to accept and establish connections from new peers.

The original implementation of peer eviction prioritized aliveness over
connection stability which made the peer count unstable for some users.

As this may cause discomfort or infrastructure alerts if stability is
tracked, adjust the eviction to be less aggressive by only evicting
peers when the node has fully stalled. This causes the node to have some
peers who are inactive and won't send any block announcements.
These nodes are removed if the local node is able to receive at least
one block announcement from one of its peers as the inactivity of the
substream is detected when a notification is sent.

If the node won't send or receive any block annoucements for 30 seconds,
it's considered stalled and it will evict all peers,
causing `ProtocolController` to accept and establish connections from new
peers.
@altonen altonen added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jul 24, 2023
@altonen altonen requested a review from a team July 24, 2023 08:57
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

Co-authored-by: Dmitry Markin <dmitry@markin.tech>
@altonen altonen requested a review from kpp July 24, 2023 14:18
@altonen
Copy link
Contributor Author

altonen commented Jul 25, 2023

@kpp please take a look

@altonen
Copy link
Contributor Author

altonen commented Aug 4, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@bkchr bkchr merged commit 5e76587 into master Aug 15, 2023
@bkchr bkchr deleted the sync-adjust-eviction-threshold branch August 15, 2023 21:47
Ank4n pushed a commit that referenced this pull request Aug 20, 2023
* Make peer evictions less aggressive

The original implementation of peer eviction prioritized aliveness over
connection stability which made the peer count unstable for some users.

As this may cause discomfort or infrastructure alerts if stability is
tracked, adjust the eviction to be less aggressive by only evicting
peers when the node has fully stalled. This causes the node to have some
peers who are inactive and won't send any block announcements.
These nodes are removed if the local node is able to receive at least
one block announcement from one of its peers as the inactivity of the
substream is detected when a notification is sent.

If the node won't send or receive any block annoucements for 30 seconds,
it's considered stalled and it will evict all peers,
causing `ProtocolController` to accept and establish connections from new
peers.

* Update client/network/sync/src/engine.rs

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

* Track last send and received notification simultaneously

---------

Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: parity-processbot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants