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

eth/downloader: fix the stall checks/drops during sync #2855

Merged
merged 1 commit into from
Jul 25, 2016

Conversation

karalabe
Copy link
Member

This is a fix for an interesting corner case regression introduced by the EDGE release. Our code before the EDGE release tried to estimate the capacity of a peer and fetch data proportional to it. If the retrieval timed out, it assumed we were a bit over zealous with te request and reduced the peer capacity to zero (i.e. 1 data item per request). If this also timed out afterwards, the peer was deemed useless beyond retention (during sync, which needs performance) and dropped it.

The EDGE release introduced a fancier throughput and latency measurement algo, which among other constantly tries to request just a bit more than the capacity. This way it can correctly separate the latency from the bandwidth. This means however, that it will never request just one data item, rather always a minimum of two. Our stall checker code however only assumed 1 to be stalling. So EDGE effectively disabled the stall drop.

The big problem with this is that if I connect to two very bad peers, which time out always, then instead of dropping them off eventually, we switch between one or the other, trying to request the same 2 data items will infinity. We'll never break out of this loop as long as they are there, blocking up sync.

@robotally
Copy link

robotally commented Jul 22, 2016

Vote Count Reviewers
👍 0
👎 0

Updated: Mon Jul 25 10:08:07 UTC 2016

@karalabe
Copy link
Member Author

@fjl Please review this sooner rather than later :)

@fjl
Copy link
Contributor

fjl commented Jul 25, 2016

👍

@fjl fjl merged commit 3e3a79e into ethereum:develop Jul 25, 2016
@fjl fjl removed the review label Jul 25, 2016
@karalabe karalabe added this to the 1.4.11 milestone Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants