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

Improve block and transaction propagation #9954

Merged
merged 10 commits into from Nov 28, 2018
Merged

Improve block and transaction propagation #9954

merged 10 commits into from Nov 28, 2018

Conversation

@tomusdrw
Copy link
Contributor

tomusdrw commented Nov 22, 2018

Closes #9576

This addresses the issue identified by @AlexeyAkhunov regarding block propagation. Before the PR blocks were propagated after they were successfully imported to the DB (which means we had to execute all transactions).

This behaviour leads to a situation that I replicated in tests by running a PoA network with one validator (Node 5) and 4 regular nodes that are connected into a chain: 5 -- 4 -- 3 -- 2 --1:

Node 1: 87 (+5873ms)
Node 2: 87 (+4430ms)
Node 3: 87 (+2896ms)
Node 4: 87 (+1439ms)
Node 5: 87 (+0ms)

Node 1: 88 (+5827ms)
Node 2: 88 (+4413ms)
Node 3: 88 (+2883ms)
Node 4: 88 (+1422ms)
Node 5: 88 (+0ms)

To address this issue we propagate blocks right after initial verification (engine::verify_block_basic), which means we verify PoW or signatures in case of PoA network. Similar mechanism for priority propagation is also applied for transactions to improve their propagation as well.

The results looks like this:

4818719 Node 1: 138 (+1685ms)
4818719 Node 2: 138 (+1674ms)
4818719 Node 3: 138 (+1667ms)
4818719 Node 4: 138 (+1688ms)
4818719 Node 5: 138 (+0ms)
 
4825999 Node 1: 139 (+1232ms)
4825999 Node 2: 139 (+1238ms)
4825999 Node 3: 139 (+1235ms)
4825999 Node 4: 139 (+1229ms)
4825999 Node 5: 139 (+0ms)

(the fluctation between 1.2s and 1.6s per block import is just caused by my machine).

This PR also improves receiving of transactions from other peers, as we don't require sync.write() lock.
The fix should lower the uncle rate.

tomusdrw added 10 commits Nov 20, 2018
@@ -583,6 +583,13 @@ impl<K: Kind> VerificationQueue<K> {
result
}

/// Returns true if there is nothing currently in the queue.
/// TODO [ToDr] Optimize to avoid locking

This comment has been minimized.

Copy link
@sorpaas

sorpaas Nov 26, 2018

Member

Would you mind to create a new issue and include an issue number for this TODO item?

@@ -67,14 +65,9 @@ pub struct SyncHandler;
impl SyncHandler {
/// Handle incoming packet from peer
pub fn on_packet(sync: &mut ChainSync, io: &mut SyncIo, peer: PeerId, packet_id: u8, data: &[u8]) {
if packet_id != STATUS_PACKET && !sync.peers.contains_key(&peer) {

This comment has been minimized.

Copy link
Member

andresilva left a comment

LGTM. Awesome results :) 🥇

@@ -228,6 +228,37 @@ impl AttachedProtocol {
}
}

/// A prioritized tasks run in a specialised timer.

This comment has been minimized.

Copy link
@andresilva

andresilva Nov 28, 2018

Member
Suggested change
/// A prioritized tasks run in a specialised timer.
/// A prioritized task runs in a specialised timer.
}
}

const PEERS_TIMER: TimerToken = 0;
const SYNC_TIMER: TimerToken = 1;
const TX_TIMER: TimerToken = 2;
const PRIORITY_TIMER: TimerToken = 3;

This comment has been minimized.

Copy link
@andresilva

andresilva Nov 28, 2018

Member

We should be careful when merging this PR with #9967 because if it doesn't conflict we end up with duplicate tokens.

@5chdn 5chdn merged commit 0b5bbf6 into master Nov 28, 2018
4 of 5 checks passed
4 of 5 checks passed
continuous-integration/gitlab-test-nightly Build stage: optional; status: failed
Details
continuous-integration/gitlab-build-android Build stage: optional; status: success
Details
continuous-integration/gitlab-cargo-audit Build stage: optional; status: success
Details
continuous-integration/gitlab-test-beta Build stage: optional; status: success
Details
continuous-integration/gitlab-test-linux Build stage: test; status: success
Details
@5chdn 5chdn deleted the td-propagation branch Nov 28, 2018
5chdn added a commit that referenced this pull request Nov 28, 2018
* 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.
5chdn added a commit that referenced this pull request Nov 29, 2018
* 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
niklasad1 added a commit that referenced this pull request Dec 16, 2018
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.