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

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
Collaborator

@tomusdrw 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 tomusdrw added A0-pleasereview 🤓 Pull request needs code review. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. labels Nov 22, 2018
@5chdn 5chdn added this to the 2.3 milestone Nov 25, 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sorpaas sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 26, 2018
This was referenced Nov 27, 2018
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Awesome results :) 🥇

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

/// A prioritized tasks run in a specialised timer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@5chdn 5chdn deleted the td-propagation branch November 28, 2018 10:30
5chdn pushed 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 pushed 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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Propagate new blocks after verifying PoW but without waiting for them to be imported
4 participants