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

Fix Waiting For Bandwidth Issue While Syncing #11853

Merged
merged 15 commits into from
Jan 12, 2023
Merged

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Jan 9, 2023

What type of PR is this?

Syncing Improvement

What does this PR do? Why is it needed?

In smaller devnets, it came up that prysm was waiting an excessive period of time for peers to replenish their limits
for our leaky bucket. We modify our bandwidth waiting method, to more accurately gauge when it is the right time to correctly
request blocks from those particular peers again. This also adds a unit test for this particular case

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas added Networking P2P related items Sync Sync (regular, initial, checkpoint) related issues labels Jan 9, 2023
@nisdas nisdas requested a review from a team as a code owner January 9, 2023 13:03
saolyn
saolyn previously approved these changes Jan 9, 2023
Copy link
Contributor

@saolyn saolyn left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 378 to 382
tillEmpty := f.rateLimiter.TillEmpty(pid.String())
blocksNeeded := int64(count - uint64(rem))
currentNumBlks := f.rateLimiter.Capacity() - rem
expectedTime := int64(tillEmpty) * blocksNeeded / currentNumBlks
timer := time.NewTimer(time.Duration(expectedTime))
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be best if we abstract this into a small helper where the right math can be tested

@rauljordan
Copy link
Contributor

tests fail @nisdas

@prylabs-bulldozer prylabs-bulldozer bot merged commit 396fc3d into develop Jan 12, 2023
@delete-merged-branch delete-merged-branch bot deleted the fixBandwidthBug branch January 12, 2023 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Networking P2P related items Sync Sync (regular, initial, checkpoint) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants