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

perf(discv5): boost bootstrap lookups #7695

Merged
merged 10 commits into from Apr 17, 2024
Merged

perf(discv5): boost bootstrap lookups #7695

merged 10 commits into from Apr 17, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Apr 17, 2024

Fills the three buckets at log2distance furthest from the local node id, at 5 second intervals, at bootstrap. This gives RLPx a better chance of finding peers instantly after bootstrapping.

@emhane emhane added C-perf A change motivated by improving speed, memory usage or disk footprint A-discv5 Related to discv5 discovery A-op-reth Related to Optimism and op-reth labels Apr 17, 2024
@emhane emhane requested a review from joshieDo April 17, 2024 10:33
Copy link
Collaborator

@joshieDo joshieDo left a comment

Choose a reason for hiding this comment

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

lgtm

would one second be too noisy? Ideally we would keep the aggressive discovery until we we connect to a peer in our chain, but can be done later on, I'm not even sure if we do that for discv4

@emhane
Copy link
Member Author

emhane commented Apr 17, 2024

lgtm

would one second be too noisy? Ideally we would keep the aggressive discovery until we we connect to a peer in our chain, but can be done later on, I'm not even sure if we do that for discv4

this is stats I have so far, from RLPx peering. all started clean (without a known_peers.json file, and new non-default port).

#7683
reverses start lookup target (start filling buckets at log2distance furthest away from local node first)
Screenshot 2024-04-17 at 15 58 31

on this branch
run 1
Screenshot 2024-04-17 at 16 27 04
run 2
Screenshot 2024-04-17 at 17 02 06

clearly this pr is better for bootstrapping, it makes a successful outgoing RLPx connection within 5 mins. think we only need to do the pulses for 100 counts (with 5 sec sleeps, 100 counts amount to 20 mins, logs show bootstrap count down becomes zero from 14:33:55 to 14:55:25). after that, main doesn't perform better than this branch in making outgoing RLPx connections.

we can expose the bootstrap pulse interval and the countdown in cli in a separate pr, then we can try 1 second like that

@emhane emhane requested a review from joshieDo April 17, 2024 15:18
@emhane
Copy link
Member Author

emhane commented Apr 17, 2024

hm, we could move the bootstrap pulse out of the loop, and stack them on top of each other without sleep, let me try that

@emhane
Copy link
Member Author

emhane commented Apr 17, 2024

hm, we could move the bootstrap pulse out of the loop, and stack them on top of each other without sleep, let me try that

tried this, it's not good. the queries need some time in-between so that the next query can use the returned peers from the last query, 5 seconds seems good.

thought query_parallelism in discv5 would be handled at a higher level, but it's actually handled inside each query, not among queries. so doesn't work if they aren't executed more or less sequentially.

@emhane emhane added this pull request to the merge queue Apr 17, 2024
@emhane emhane removed this pull request from the merge queue due to a manual request Apr 17, 2024
@emhane emhane added this pull request to the merge queue Apr 17, 2024
Merged via the queue into main with commit 9557ce0 Apr 17, 2024
27 checks passed
@emhane emhane deleted the emhane/boost-bootstrap branch April 17, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-discv5 Related to discv5 discovery A-op-reth Related to Optimism and op-reth 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.

None yet

2 participants