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
[Merged by Bors] - fetch: requests got lost when there are no peers #4849
[Merged by Bors] - fetch: requests got lost when there are no peers #4849
Conversation
updates: rlayers( | ||
rlayer(start, rblock(idg("1"), fixture.Valid(), fixture.Data())), | ||
rlayer(start+1, rblock(idg("2"), fixture.Valid(), fixture.Data())), | ||
rlayer(start+2, rblock(idg("3"), fixture.Valid())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe extend it with one more block with data? and one more call that both got applied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fetch/fetch.go
Outdated
log.Stringer("hash", msg.Hash), | ||
log.String("hint", string(msg.Hint)), | ||
) | ||
f.unprocessed[msg.Hash] = req |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so GetBlocks will not return immediately with error if no peers, but instead we will do the request when peers are available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. essentially put those requests back to the queue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you prefer to fail all in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this approach is fine for me too, hard to tell what is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually i prefer returning error right away. it's easier for debugging (and avoiding future bug)
Codecov Report
@@ Coverage Diff @@
## develop #4849 +/- ##
=========================================
- Coverage 76.9% 76.8% -0.1%
=========================================
Files 261 261
Lines 29689 29720 +31
=========================================
+ Hits 22831 22850 +19
- Misses 5406 5418 +12
Partials 1452 1452
|
bors try |
tryBuild failed: |
bors merge |
## Motivation requests are not restored back to the queue when there were no peers to dispatch to. ## Changes - restore requests when there are no peers - push mesh to still make progress even when there are missing blocks - increase frequency of state sync ## testing i manually test against mainnet, artificially causing GetPeers() to return zero peers. mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.
Build failed: |
bors merge |
## Motivation requests are not restored back to the queue when there were no peers to dispatch to. ## Changes - restore requests when there are no peers - push mesh to still make progress even when there are missing blocks - increase frequency of state sync ## testing i manually test against mainnet, artificially causing GetPeers() to return zero peers. mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.
Pull request successfully merged into develop. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
requests are not restored back to the queue when there were no peers to dispatch to. - restore requests when there are no peers - push mesh to still make progress even when there are missing blocks - increase frequency of state sync i manually test against mainnet, artificially causing GetPeers() to return zero peers. mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.
requests are not restored back to the queue when there were no peers to dispatch to. - restore requests when there are no peers - push mesh to still make progress even when there are missing blocks - increase frequency of state sync i manually test against mainnet, artificially causing GetPeers() to return zero peers. mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.
requests are not restored back to the queue when there were no peers to dispatch to. - restore requests when there are no peers - push mesh to still make progress even when there are missing blocks - increase frequency of state sync i manually test against mainnet, artificially causing GetPeers() to return zero peers. mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.
Motivation
requests are not restored back to the queue when there were no peers to dispatch to.
Changes
testing
i manually test against mainnet, artificially causing GetPeers() to return zero peers.
mesh was making very slow progress because it aborts as soon as it finds a missing block, not executing the next block even when it's available.