Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add a timeout for light client sync requests #7848

Merged
merged 2 commits into from
Feb 14, 2018

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Feb 9, 2018

Fix #6319

This PR adds a 7 seconds timeout when we request a header range from a remote. If the timeout expires, the request will be re-assigned, most likely to a different node.

My machine is still syncing with this patch right now, but I've successfully synced more than 2 million blocks without dropping to 0 hdr/s at any point.

EDIT: Briefly dropped to 0 hdr/s around the 3 millionth block, but I think it's because my Internet connection briefly dropped. Sync resumed 10 seconds later.

EDIT2: Fully sync'ed from the beginning without any interruption except the one of my first edit.

@tomaka tomaka added A0-pleasereview 🤓 Pull request needs code review. B0-patch M4-core ⛓ Core client code / Rust. labels Feb 9, 2018
@parity-cla-bot
Copy link

It looks like @tomaka signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@tomaka
Copy link
Contributor Author

tomaka commented Feb 9, 2018

The CI failure is in secret store and looks unrelated to my change.

@tomaka
Copy link
Contributor Author

tomaka commented Feb 11, 2018

I tried two other full synchronizations over the week end, and unfortunately the second one has gotten stuck, meaning that the issue isn't entirely solved :-/

@rphmeier
Copy link
Contributor

The network service already handles timeouts here: https://github.com/paritytech/parity/blob/master/ethcore/light/src/net/mod.rs#L88

although for groups of 256 headers I think it will end up timing out after a minute which is far too long. It should really be mapped onto something like a linear function something like 7 + 0.01x where x is the number of headers requested.

@5chdn 5chdn added this to the 1.10 milestone Feb 12, 2018
@5chdn
Copy link
Contributor

5chdn commented Feb 13, 2018

Just tested, this does indeed resolve #6319 on first shot within 2 hours.

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 14, 2018
@5chdn 5chdn merged commit bf57eb8 into openethereum:master Feb 14, 2018
@tomaka tomaka deleted the light-client-sync-timeout branch February 14, 2018 10:59
tomusdrw pushed a commit that referenced this pull request Feb 14, 2018
* Add a timeout for light client sync requests

* Adjusting timeout to number of headers
5chdn pushed a commit that referenced this pull request Feb 14, 2018
* update back-references more aggressively after answering from cache (#7578)

* Add new EF ropstens nodes. (#7824)

* Add new EF ropstens nodes.

* Fix tests

* Add a timeout for light client sync requests (#7848)

* Add a timeout for light client sync requests

* Adjusting timeout to number of headers

* Flush keyfiles. Resolves #7632 (#7868)

* Fix wallet import (#7873)

* rpc: generate new account id for imported wallets

* ethstore: handle duplicate wallet filenames

* ethstore: simplify deduplication of wallet file names

* ethstore: do not dedup wallet filenames on update

* ethstore: fix minor grumbles

* [WASM] mem_cmp added to the Wasm runtime (#7539)

* mem_cmp added to the Wasm runtime

* schedule.wasm.mem_copy to schedule.wasm.mem_cmp for mem_cmp

* [Wasm] memcmp fix and test added (#7590)

* [Wasm] memcmp fix and test added

* [Wasm] use reqrep_test! macro for memcmp test

* wasmi interpreter (#7796)

* adjust storage update evm-style (#7812)

* disable internal memory (#7842)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants