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

Avoid ordering peers based on peerid in blockfetch #3535

Merged
merged 2 commits into from Jan 20, 2022

Conversation

karknu
Copy link
Contributor

@karknu karknu commented Dec 9, 2021

Peers where ordered based on SockAddr, which generally meant that all
peers would pick the same second choice peer to download blocks from in
Deadline mode. And the same third pick and so on.

This changes ranking so that peers are sorted based on G.

@karknu karknu requested a review from coot as a code owner December 9, 2021 07:07
@karknu karknu force-pushed the karknu/blockfetch_order_p2p_master_1.31.0 branch 3 times, most recently from 5e96fbf to bedc555 Compare January 7, 2022 17:17
@karknu karknu force-pushed the karknu/blockfetch_order_p2p_master_1.31.0 branch from bedc555 to 5463dcf Compare January 17, 2022 16:49
@karknu karknu force-pushed the karknu/blockfetch_order_p2p_master_1.31.0 branch from 5463dcf to 7d18f64 Compare January 19, 2022 08:15
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

lgtm

@coot
Copy link
Contributor

coot commented Jan 20, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Jan 20, 2022
3535: Avoid ordering peers based on peerid in blockfetch r=coot a=karknu

Peers where ordered based on SockAddr, which generally meant that all
peers would pick the same second choice peer to download blocks from in
Deadline mode. And the same third pick and so on.

This changes ranking so that peers are sorted based on G.

Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>
@karknu
Copy link
Contributor Author

karknu commented Jan 20, 2022

bors cancel

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 20, 2022

Canceled.

Peers where ordered based on SockAddr, which generally meant that all
peers would pick the same second choice peer to download blocks from in
Deadline mode. And the same third pick and so on.

This changes ranking so that peers are sorted based on G.
In case two peer's G where close to each other the ordering wasn't
stable. This fixes the ordering and adds matching tests.
@karknu karknu force-pushed the karknu/blockfetch_order_p2p_master_1.31.0 branch from 7d18f64 to 8bb40ce Compare January 20, 2022 14:52
@karknu karknu changed the base branch from p2p-master-1.31.0 to master January 20, 2022 14:53
@karknu
Copy link
Contributor Author

karknu commented Jan 20, 2022

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 20, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit ea1989e into master Jan 20, 2022
@iohk-bors iohk-bors bot deleted the karknu/blockfetch_order_p2p_master_1.31.0 branch January 20, 2022 16:31
@karknu
Copy link
Contributor Author

karknu commented Jan 20, 2022

Fixes #3536

karknu added a commit that referenced this pull request Jan 21, 2022
3535: Avoid ordering peers based on peerid in blockfetch r=karknu a=karknu

Peers where ordered based on SockAddr, which generally meant that all
peers would pick the same second choice peer to download blocks from in
Deadline mode. And the same third pick and so on.

This changes ranking so that peers are sorted based on G.

Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>
@karknu karknu mentioned this pull request Jan 21, 2022
4 tasks
coot pushed a commit that referenced this pull request May 16, 2022
3535: Avoid ordering peers based on peerid in blockfetch r=karknu a=karknu

Peers where ordered based on SockAddr, which generally meant that all
peers would pick the same second choice peer to download blocks from in
Deadline mode. And the same third pick and so on.

This changes ranking so that peers are sorted based on G.

Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>
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.

None yet

2 participants