Fix parallel transactions race-condition (#10995)

* WiP : clear pending txs cache & tick in Miner

* Fixed pending transactions

* Revert debugging code

* Add ToDo comment

* Remove commented-out code

* Reverse LTO setting

* WiP

* Try to seal a new block if there are pending transactions

* Try resealing only for internal imports

* Remove logging

* Use AtomicU64 instead of Mutex<BlockNumber>

* Remove TxQueue cache clear // Update AtomicUint logic

* Update comments in Miner

* Revert import of `parking_lot`

* Update `transaction-pool` dependency

* Call directly `update_sealing`

* Call `update_sealing` directly
This commit is contained in:
Nicolas Gotchac 2019-09-11 18:44:31 +02:00 committed by GitHub
parent 240ef46c72
commit 5e2def1b23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 34 additions and 15 deletions

10
Cargo.lock generated
View File

@ -1263,7 +1263,7 @@ dependencies = [
"serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)",
"serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)",
"trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
] ]
@ -1366,7 +1366,7 @@ dependencies = [
"time-utils 0.1.0", "time-utils 0.1.0",
"tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
"trace 0.1.0", "trace 0.1.0",
"transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
"url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
"vm 0.1.0", "vm 0.1.0",
@ -3141,7 +3141,7 @@ dependencies = [
"tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
"tokio-timer 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-timer 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"trace 0.1.0", "trace 0.1.0",
"transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"verification 0.1.0", "verification 0.1.0",
"vm 0.1.0", "vm 0.1.0",
@ -4737,7 +4737,7 @@ dependencies = [
[[package]] [[package]]
name = "transaction-pool" name = "transaction-pool"
version = "2.0.0" version = "2.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [ dependencies = [
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
@ -5568,7 +5568,7 @@ dependencies = [
"checksum toml 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b8c96d7873fa7ef8bdeb3a9cda3ac48389b4154f32b9803b4bc26220b677b039" "checksum toml 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b8c96d7873fa7ef8bdeb3a9cda3ac48389b4154f32b9803b4bc26220b677b039"
"checksum toolshed 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "450441e131c7663af72e63a33c02a6a1fbaaa8601dc652ed6757813bb55aeec7" "checksum toolshed 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "450441e131c7663af72e63a33c02a6a1fbaaa8601dc652ed6757813bb55aeec7"
"checksum trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe82f2f0bf1991e163e757baf044282823155dd326e70f44ce2186c3c320cc9" "checksum trace-time 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe82f2f0bf1991e163e757baf044282823155dd326e70f44ce2186c3c320cc9"
"checksum transaction-pool 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8d8bd3123931aa6e49dd03bc8a2400490e14701d779458d1f1fff1f04c6f666" "checksum transaction-pool 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "454adc482e32785c3beab9415dd0f3c689f29cc2d16717eb62f6a784d53544b4"
"checksum transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "aeb4b191d033a35edfce392a38cdcf9790b6cebcb30fa690c312c29da4dc433e" "checksum transient-hashmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "aeb4b191d033a35edfce392a38cdcf9790b6cebcb30fa690c312c29da4dc433e"
"checksum trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b65d609ae631d808c6c1cc23a622733d5a0b66a7d67e9f5cd5171562a1f4cb5" "checksum trie-db 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b65d609ae631d808c6c1cc23a622733d5a0b66a7d67e9f5cd5171562a1f4cb5"
"checksum trie-standardmap 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "64fda153c00484d640bc91334624be22ead0e5baca917d9fd53ff29bdebcf9b2" "checksum trie-standardmap 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)" = "64fda153c00484d640bc91334624be22ead0e5baca917d9fd53ff29bdebcf9b2"

View File

@ -14,6 +14,8 @@
// You should have received a copy of the GNU General Public License // You should have received a copy of the GNU General Public License
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>. // along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.
use std::sync::atomic::{AtomicU64, Ordering};
use common_types::{ use common_types::{
header::Header, header::Header,
engines::{ engines::{
@ -51,6 +53,7 @@ impl From<ethjson::spec::InstantSealParams> for InstantSealParams {
pub struct InstantSeal { pub struct InstantSeal {
params: InstantSealParams, params: InstantSealParams,
machine: Machine, machine: Machine,
last_sealed_block: AtomicU64,
} }
impl InstantSeal { impl InstantSeal {
@ -59,6 +62,7 @@ impl InstantSeal {
InstantSeal { InstantSeal {
params, params,
machine, machine,
last_sealed_block: AtomicU64::new(0),
} }
} }
} }
@ -71,12 +75,20 @@ impl Engine for InstantSeal {
fn sealing_state(&self) -> SealingState { SealingState::Ready } fn sealing_state(&self) -> SealingState { SealingState::Ready }
fn generate_seal(&self, block: &ExecutedBlock, _parent: &Header) -> Seal { fn generate_seal(&self, block: &ExecutedBlock, _parent: &Header) -> Seal {
if block.transactions.is_empty() { if !block.transactions.is_empty() {
Seal::None let block_number = block.header.number();
} else { let last_sealed_block = self.last_sealed_block.load(Ordering::SeqCst);
Seal::Regular(Vec::new()) // Return a regular seal if the given block is _higher_ than
// the last sealed one
if block_number > last_sealed_block {
let prev_last_sealed_block = self.last_sealed_block.compare_and_swap(last_sealed_block, block_number, Ordering::SeqCst);
if prev_last_sealed_block == last_sealed_block {
return Seal::Regular(Vec::new())
} }
} }
}
Seal::None
}
fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> {
Ok(()) Ok(())

View File

@ -47,7 +47,7 @@ state-db = { path = "../state-db" }
time-utils = { path = "../../util/time-utils" } time-utils = { path = "../../util/time-utils" }
tiny-keccak = "1.4" tiny-keccak = "1.4"
trace = { path = "../trace" } trace = { path = "../trace" }
transaction-pool = "2.0" transaction-pool = "2.0.1"
url = "1" url = "1"
vm = { path = "../vm" } vm = { path = "../vm" }

View File

@ -33,7 +33,7 @@ use futures::sync::mpsc;
use io::IoChannel; use io::IoChannel;
use miner::filter_options::{FilterOptions, FilterOperator}; use miner::filter_options::{FilterOptions, FilterOperator};
use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache}; use miner::pool_client::{PoolClient, CachedNonceClient, NonceCache};
use miner; use miner::{self, MinerService};
use parking_lot::{Mutex, RwLock}; use parking_lot::{Mutex, RwLock};
use rayon::prelude::*; use rayon::prelude::*;
use types::{ use types::{
@ -58,7 +58,7 @@ use using_queue::{UsingQueue, GetAction};
use block::{ClosedBlock, SealedBlock}; use block::{ClosedBlock, SealedBlock};
use client::{BlockProducer, SealedBlockImporter, Client}; use client::{BlockProducer, SealedBlockImporter, Client};
use client_traits::{BlockChain, ChainInfo, Nonce, TransactionInfo}; use client_traits::{BlockChain, ChainInfo, EngineClient, Nonce, TransactionInfo};
use engine::{Engine, signer::EngineSigner}; use engine::{Engine, signer::EngineSigner};
use machine::executive::contract_address; use machine::executive::contract_address;
use spec::Spec; use spec::Spec;
@ -859,9 +859,9 @@ impl Miner {
false false
} }
} }
/// Prepare pending block, check whether sealing is needed, and then update sealing. /// Prepare pending block, check whether sealing is needed, and then update sealing.
fn prepare_and_update_sealing<C: miner::BlockChainClient>(&self, chain: &C) { fn prepare_and_update_sealing<C: miner::BlockChainClient>(&self, chain: &C) {
use miner::MinerService;
match self.engine.sealing_state() { match self.engine.sealing_state() {
SealingState::Ready => { SealingState::Ready => {
self.maybe_enable_sealing(); self.maybe_enable_sealing();
@ -1429,6 +1429,9 @@ impl miner::MinerService for Miner {
service_transaction_checker.as_ref(), service_transaction_checker.as_ref(),
); );
queue.cull(client); queue.cull(client);
if is_internal_import {
chain.update_sealing();
}
}; };
if let Err(e) = channel.send(ClientIoMessage::<Client>::execute(cull)) { if let Err(e) = channel.send(ClientIoMessage::<Client>::execute(cull)) {
@ -1436,8 +1439,12 @@ impl miner::MinerService for Miner {
} }
} else { } else {
self.transaction_queue.cull(client); self.transaction_queue.cull(client);
if is_internal_import {
self.update_sealing(chain);
} }
} }
}
if let Some(ref service_transaction_checker) = self.service_transaction_checker { if let Some(ref service_transaction_checker) = self.service_transaction_checker {
match service_transaction_checker.refresh_cache(chain) { match service_transaction_checker.refresh_cache(chain) {
Ok(true) => { Ok(true) => {

View File

@ -34,7 +34,7 @@ serde = "1.0"
serde_derive = "1.0" serde_derive = "1.0"
serde_json = "1.0" serde_json = "1.0"
trace-time = "0.1" trace-time = "0.1"
transaction-pool = "2.0" transaction-pool = "2.0.1"
[dev-dependencies] [dev-dependencies]
env_logger = "0.5" env_logger = "0.5"

View File

@ -80,7 +80,7 @@ fake-fetch = { path = "../util/fake-fetch" }
macros = { path = "../util/macros" } macros = { path = "../util/macros" }
spec = { path = "../ethcore/spec" } spec = { path = "../ethcore/spec" }
pretty_assertions = "0.1" pretty_assertions = "0.1"
transaction-pool = "2.0" transaction-pool = "2.0.1"
verification = { path = "../ethcore/verification" } verification = { path = "../ethcore/verification" }
[features] [features]