From 2234b368794ada886ead1d3e6e2fc92dd58315c2 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Thu, 17 Oct 2024 00:23:01 +0000 Subject: [PATCH] remove parent_block_id from BlockExecutor::pre_commit_block() arguments --- consensus/src/execution_pipeline.rs | 9 ++------- consensus/src/state_computer.rs | 6 +----- consensus/src/state_computer_tests.rs | 6 +----- .../src/ledger_update_stage.rs | 5 +---- .../src/transaction_committer.rs | 7 ++----- execution/executor-types/src/lib.rs | 10 ++-------- execution/executor/src/block_executor.rs | 18 ++++-------------- .../executor/src/tests/chunk_executor_tests.rs | 4 +--- execution/executor/src/tests/mod.rs | 8 ++------ 9 files changed, 16 insertions(+), 57 deletions(-) diff --git a/consensus/src/execution_pipeline.rs b/consensus/src/execution_pipeline.rs index b757bd8b0f76d7..ee11b60e12a5e8 100644 --- a/consensus/src/execution_pipeline.rs +++ b/consensus/src/execution_pipeline.rs @@ -293,7 +293,7 @@ impl ExecutionPipeline { let executor = executor.clone(); Box::pin(async move { tokio::task::spawn_blocking(move || { - executor.pre_commit_block(block_id, parent_block_id) + executor.pre_commit_block(block_id) }) .await .expect("failed to spawn_blocking")?; @@ -307,7 +307,6 @@ impl ExecutionPipeline { pre_commit_tx .send(PreCommitCommand { block_id, - parent_block_id, pre_commit_hook_fut, result_tx: pre_commit_result_tx, lifetime_guard, @@ -335,7 +334,6 @@ impl ExecutionPipeline { ) { while let Some(PreCommitCommand { block_id, - parent_block_id, pre_commit_hook_fut, result_tx, lifetime_guard, @@ -346,9 +344,7 @@ impl ExecutionPipeline { let executor = executor.clone(); monitor!( "pre_commit", - tokio::task::spawn_blocking(move || { - executor.pre_commit_block(block_id, parent_block_id) - }) + tokio::task::spawn_blocking(move || { executor.pre_commit_block(block_id) }) ) .await .expect("Failed to spawn_blocking().")?; @@ -402,7 +398,6 @@ struct LedgerApplyCommand { struct PreCommitCommand { block_id: HashValue, - parent_block_id: HashValue, pre_commit_hook_fut: BoxFuture<'static, ()>, result_tx: oneshot::Sender>, lifetime_guard: CountedRequest<()>, diff --git a/consensus/src/state_computer.rs b/consensus/src/state_computer.rs index 9bad3d4c26a472..fb8230e013c008 100644 --- a/consensus/src/state_computer.rs +++ b/consensus/src/state_computer.rs @@ -516,11 +516,7 @@ async fn test_commit_sync_race() { todo!() } - fn pre_commit_block( - &self, - _block_id: HashValue, - _parent_block_id: HashValue, - ) -> ExecutorResult<()> { + fn pre_commit_block(&self, _block_id: HashValue) -> ExecutorResult<()> { todo!() } diff --git a/consensus/src/state_computer_tests.rs b/consensus/src/state_computer_tests.rs index 7129c7a3ce28f1..f9979f99410244 100644 --- a/consensus/src/state_computer_tests.rs +++ b/consensus/src/state_computer_tests.rs @@ -144,11 +144,7 @@ impl BlockExecutorTrait for DummyBlockExecutor { Ok(StateComputeResult::new_dummy_with_input_txns(txns)) } - fn pre_commit_block( - &self, - _block_id: HashValue, - _parent_block_id: HashValue, - ) -> ExecutorResult<()> { + fn pre_commit_block(&self, _block_id: HashValue) -> ExecutorResult<()> { Ok(()) } diff --git a/execution/executor-benchmark/src/ledger_update_stage.rs b/execution/executor-benchmark/src/ledger_update_stage.rs index ceb2f54fa793b5..43aeb328a69ce8 100644 --- a/execution/executor-benchmark/src/ledger_update_stage.rs +++ b/execution/executor-benchmark/src/ledger_update_stage.rs @@ -75,10 +75,7 @@ where output.root_hash(), self.version, ); - let parent_block_id = self.executor.committed_block_id(); - self.executor - .pre_commit_block(block_id, parent_block_id) - .unwrap(); + self.executor.pre_commit_block(block_id).unwrap(); self.executor.commit_ledger(ledger_info_with_sigs).unwrap(); }, CommitProcessing::Skip => {}, diff --git a/execution/executor-benchmark/src/transaction_committer.rs b/execution/executor-benchmark/src/transaction_committer.rs index 64f5f689433fa6..e3aa70b7c534b9 100644 --- a/execution/executor-benchmark/src/transaction_committer.rs +++ b/execution/executor-benchmark/src/transaction_committer.rs @@ -85,12 +85,9 @@ where .inc_by(num_txns as u64); self.version += num_txns as u64; - let commit_start = std::time::Instant::now(); + let commit_start = Instant::now(); let ledger_info_with_sigs = gen_li_with_sigs(block_id, root_hash, self.version); - let parent_block_id = self.executor.committed_block_id(); - self.executor - .pre_commit_block(block_id, parent_block_id) - .unwrap(); + self.executor.pre_commit_block(block_id).unwrap(); self.executor.commit_ledger(ledger_info_with_sigs).unwrap(); report_block( diff --git a/execution/executor-types/src/lib.rs b/execution/executor-types/src/lib.rs index f878723e531653..b0b1dc81987bfe 100644 --- a/execution/executor-types/src/lib.rs +++ b/execution/executor-types/src/lib.rs @@ -160,19 +160,13 @@ pub trait BlockExecutorTrait: Send + Sync { block_ids: Vec, ledger_info_with_sigs: LedgerInfoWithSignatures, ) -> ExecutorResult<()> { - let mut parent_block_id = self.committed_block_id(); for block_id in block_ids { - self.pre_commit_block(block_id, parent_block_id)?; - parent_block_id = block_id; + self.pre_commit_block(block_id)?; } self.commit_ledger(ledger_info_with_sigs) } - fn pre_commit_block( - &self, - block_id: HashValue, - parent_block_id: HashValue, - ) -> ExecutorResult<()>; + fn pre_commit_block(&self, block_id: HashValue) -> ExecutorResult<()>; fn commit_ledger(&self, ledger_info_with_sigs: LedgerInfoWithSignatures) -> ExecutorResult<()>; diff --git a/execution/executor/src/block_executor.rs b/execution/executor/src/block_executor.rs index 4c082baeace572..72426a15dd951c 100644 --- a/execution/executor/src/block_executor.rs +++ b/execution/executor/src/block_executor.rs @@ -145,18 +145,14 @@ where .ledger_update(block_id, parent_block_id, state_checkpoint_output) } - fn pre_commit_block( - &self, - block_id: HashValue, - parent_block_id: HashValue, - ) -> ExecutorResult<()> { + fn pre_commit_block(&self, block_id: HashValue) -> ExecutorResult<()> { let _guard = CONCURRENCY_GAUGE.concurrency_with(&["block", "pre_commit_block"]); self.inner .read() .as_ref() .expect("BlockExecutor is not reset") - .pre_commit_block(block_id, parent_block_id) + .pre_commit_block(block_id) } fn commit_ledger(&self, ledger_info_with_sigs: LedgerInfoWithSignatures) -> ExecutorResult<()> { @@ -338,20 +334,14 @@ where Ok(block.output.expect_complete_result()) } - fn pre_commit_block( - &self, - block_id: HashValue, - parent_block_id: HashValue, - ) -> ExecutorResult<()> { + fn pre_commit_block(&self, block_id: HashValue) -> ExecutorResult<()> { let _timer = COMMIT_BLOCKS.start_timer(); info!( LogSchema::new(LogEntry::BlockExecutor).block_id(block_id), "pre_commit_block", ); - let mut blocks = self.block_tree.get_blocks(&[parent_block_id, block_id])?; - let block = blocks.pop().expect("guaranteed"); - let _parent_block = blocks.pop().expect("guaranteed"); + let block = self.block_tree.get_block(block_id)?; fail_point!("executor::pre_commit_block", |_| { Err(anyhow::anyhow!("Injected error in pre_commit_block.").into()) diff --git a/execution/executor/src/tests/chunk_executor_tests.rs b/execution/executor/src/tests/chunk_executor_tests.rs index f7e7154aac8be9..46de72e5d06fde 100644 --- a/execution/executor/src/tests/chunk_executor_tests.rs +++ b/execution/executor/src/tests/chunk_executor_tests.rs @@ -344,9 +344,7 @@ fn commit_1_pre_commit_2_return_3() -> ( output.root_hash(), ledger_info.ledger_info().transaction_accumulator_hash() ); - block_executor - .pre_commit_block(block_id, parent_block_id) - .unwrap(); + block_executor.pre_commit_block(block_id).unwrap(); parent_block_id = block_id; } // commit till block 1 diff --git a/execution/executor/src/tests/mod.rs b/execution/executor/src/tests/mod.rs index 426a7b644e7d7e..e34cab4e0eccb0 100644 --- a/execution/executor/src/tests/mod.rs +++ b/execution/executor/src/tests/mod.rs @@ -291,9 +291,7 @@ fn test_executor_commit_twice() { ) .unwrap(); let ledger_info = gen_ledger_info(6, output1.root_hash(), block1_id, 1); - executor - .pre_commit_block(block1_id, executor.committed_block_id()) - .unwrap(); + executor.pre_commit_block(block1_id).unwrap(); executor.commit_ledger(ledger_info.clone()).unwrap(); executor.commit_ledger(ledger_info).unwrap(); } @@ -388,9 +386,7 @@ fn create_blocks_and_chunks( ) .unwrap(); assert_eq!(output.version(), version); - block_executor - .pre_commit_block(block_id, parent_block_id) - .unwrap(); + block_executor.pre_commit_block(block_id).unwrap(); let ledger_info = gen_ledger_info(version, output.root_hash(), block_id, version); out_blocks.push((txns, ledger_info)); parent_block_id = block_id;