Fix unstable peers and slowness in sync by ngotchac · Pull Request #9967 · openethereum/parity-ethereum

ngotchac · 2018-11-26T18:06:09Z

This PR fixes some issues with the stability of peer counts, and overall slowness in block sync.

Here are graphs that compares block height against time for the current stable and this branch:

Here for the peer counts:

The issue was that sync.continue_sync was called every time a packet was received, so it had to go through the whole list of peers, randomize it, etc.. Which could be quite expensive when connected to a lot of peers.

Now it is called on a timer, every 2.5 seconds.

sorpaas

LGTM!

Some tests would need to be fixed. Seems we can just add call continue_sync in sync::tests::helpers::EthPeer::sync_step.

tomusdrw

Amazing 🎉! Great work @ngotchac!

andresilva

LGTM! Great job 🥇 :)

andresilva · 2018-11-27T21:09:11Z

Should we backport this as well?

Co-Authored-By: ngotchac <ngotchac@gmail.com>

* Don't sync all peers after each response * Update formating * Fix tests: add `continue_sync` to `Sync_step` * Update ethcore/sync/src/chain/mod.rs Co-Authored-By: ngotchac <ngotchac@gmail.com>

folsen · 2018-11-29T09:57:40Z

@ngotchac just out of curiosity, how did you generate the graphs?

* version: bump beta to 2.2.2 * Add experimental RPCs flag (#9928) * WiP * Enable experimental RPCs. * Keep existing blocks when restoring a Snapshot (#8643) * Rename db_restore => client * First step: make it compile! * Second step: working implementation! * Refactoring * Fix tests * PR Grumbles * PR Grumbles WIP * Migrate ancient blocks interating backward * Early return in block migration if snapshot is aborted * Remove RwLock getter (PR Grumble I) * Remove dependency on `Client`: only used Traits * Add test for recovering aborted snapshot recovery * Add test for migrating old blocks * Fix build * PR Grumble I * PR Grumble II * PR Grumble III * PR Grumble IV * PR Grumble V * PR Grumble VI * Fix one test * Fix test * PR Grumble * PR Grumbles * PR Grumbles II * Fix tests * Release RwLock earlier * Revert Cargo.lock * Update _update ancient block_ logic: set local in `commit` * Update typo in ethcore/src/snapshot/service.rs Co-Authored-By: ngotchac <ngotchac@gmail.com> * Adjust requests costs for light client (#9925) * PIP Table Cost relative to average peers instead of max peers * Add tracing in PIP new_cost_table * Update stat peer_count * Use number of leeching peers for Light serve costs * Fix test::light_params_load_share_depends_on_max_peers (wrong type) * Remove (now) useless test * Remove `load_share` from LightParams.Config Prevent div. by 0 * Add LEECHER_COUNT_FACTOR * PR Grumble: u64 to u32 for f64 casting * Prevent u32 overflow for avg_peer_count * Add tests for LightSync::Statistics * Fix empty steps (#9939) * Don't send empty step twice or empty step then block. * Perform basic validation of locally sealed blocks. * Don't include empty step twice. * prevent silent errors in daemon mode, closes #9367 (#9946) * Fix a deadlock (#9952) * Update informant: - decimal in Mgas/s - print every 5s (not randomly between 5s and 10s) * Fix dead-lock in `blockchain.rs` * Update locks ordering * Fix light client informant while syncing (#9932) * Add `is_idle` to LightSync to check importing status * Use SyncStateWrapper to make sure is_idle gets updates * Update is_major_import to use verified queue size as well * Add comment for `is_idle` * Add Debug to `SyncStateWrapper` * `fn get` -> `fn into_inner` * ci: rearrange pipeline by logic (#9970) * ci: rearrange pipeline by logic * ci: rename docs script * fix docker build (#9971) * Deny unknown fields for chainspec (#9972) * Add deny_unknown_fields to chainspec * Add tests and fix existing one * Remove serde_ignored dependency for chainspec * Fix rpc test eth chain spec * Fix starting_nonce_test spec * Improve block and transaction propagation (#9954) * Refactor sync to add priority tasks. * Send priority tasks notifications. * Propagate blocks, optimize transactions. * Implement transaction propagation. Use sync_channel. * Tone down info. * Prevent deadlock by not waiting forever for sync lock. * Fix lock order. * Don't use sync_channel to prevent deadlocks. * Fix tests. * Fix unstable peers and slowness in sync (#9967) * Don't sync all peers after each response * Update formating * Fix tests: add `continue_sync` to `Sync_step` * Update ethcore/sync/src/chain/mod.rs Co-Authored-By: ngotchac <ngotchac@gmail.com> * fix rpc middlewares * fix Cargo.lock * json: resolve merge in spec * rpc: fix starting_nonce_test * ci: allow nightl job to fail

ngotchac · 2018-11-29T10:10:30Z

@folsen I wrote a small Rust script that runs the given binary with a given config basically, then prints the graphs using gnuplot. Are you interested? I can publish it on GH

folsen · 2018-11-29T10:15:06Z

@ngotchac definitely interested, seems to cross over with what some of the QA guys are working on

ngotchac · 2018-12-11T10:35:26Z

The script used to get the graphs : https://github.com/ngotchac/eth-metrics
It's very PoC for now.

* Don't sync all peers after each response * Update formating * Fix tests: add `continue_sync` to `Sync_step` * Update ethcore/sync/src/chain/mod.rs Co-Authored-By: ngotchac <ngotchac@gmail.com>

Don't sync all peers after each response

60b16be

ngotchac added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 26, 2018

sorpaas approved these changes Nov 26, 2018

View reviewed changes

Comment thread ethcore/sync/src/api.rs Outdated

Comment thread ethcore/sync/src/chain/mod.rs Outdated

Comment thread ethcore/sync/src/chain/mod.rs Outdated

sorpaas added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 26, 2018

5chdn added this to the 2.3 milestone Nov 26, 2018

Update formating

6b86de9

5chdn added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Nov 27, 2018

Fix tests: add continue_sync to Sync_step

a33b54a

sorpaas reviewed Nov 27, 2018

View reviewed changes

Comment thread ethcore/sync/src/chain/mod.rs Outdated

sorpaas added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 27, 2018

tomusdrw approved these changes Nov 27, 2018

View reviewed changes

andresilva approved these changes Nov 27, 2018

View reviewed changes

andresilva mentioned this pull request Nov 28, 2018

Improve block and transaction propagation #9954

Merged

sorpaas and others added 2 commits November 28, 2018 11:33

Update ethcore/sync/src/chain/mod.rs

e3fd6b0

Co-Authored-By: ngotchac <ngotchac@gmail.com>

Merge branch 'master' into ng-stable-peers-2

4d7c2c3

sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Nov 28, 2018

5chdn added the B1-patch-beta 🕷🕷 label Nov 28, 2018

5chdn merged commit 5e9dc18 into master Nov 28, 2018

5chdn deleted the ng-stable-peers-2 branch November 28, 2018 11:19

5chdn mentioned this pull request Nov 28, 2018

Backports for beta 2.2.2 #9976

Merged

12 tasks

ngotchac mentioned this pull request Jan 14, 2019

"TooManyPeers" when there are only 5 peers on private blockchain #7203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unstable peers and slowness in sync#9967

Fix unstable peers and slowness in sync#9967
5chdn merged 5 commits into
masterfrom
ng-stable-peers-2

ngotchac commented Nov 26, 2018

Uh oh!

sorpaas left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomusdrw left a comment

Uh oh!

andresilva left a comment

Uh oh!

andresilva commented Nov 27, 2018

Uh oh!

folsen commented Nov 29, 2018

Uh oh!

ngotchac commented Nov 29, 2018

Uh oh!

folsen commented Nov 29, 2018

Uh oh!

ngotchac commented Dec 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ngotchac commented Nov 26, 2018

Uh oh!

sorpaas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomusdrw left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva commented Nov 27, 2018

Uh oh!

folsen commented Nov 29, 2018

Uh oh!

ngotchac commented Nov 29, 2018

Uh oh!

folsen commented Nov 29, 2018

Uh oh!

ngotchac commented Dec 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sorpaas left a comment •

edited

Loading