Skip to content

Commit

Permalink
remove parent_block_id from BlockExecutor::pre_commit_block() arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
msmouse committed Oct 17, 2024
1 parent 2b16fe4 commit 78b3331
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 57 deletions.
9 changes: 2 additions & 7 deletions consensus/src/execution_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand All @@ -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,
Expand Down Expand Up @@ -335,7 +334,6 @@ impl ExecutionPipeline {
) {
while let Some(PreCommitCommand {
block_id,
parent_block_id,
pre_commit_hook_fut,
result_tx,
lifetime_guard,
Expand All @@ -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().")?;
Expand Down Expand Up @@ -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<ExecutorResult<()>>,
lifetime_guard: CountedRequest<()>,
Expand Down
6 changes: 1 addition & 5 deletions consensus/src/state_computer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
}

Expand Down
6 changes: 1 addition & 5 deletions consensus/src/state_computer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
5 changes: 1 addition & 4 deletions execution/executor-benchmark/src/ledger_update_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {},
Expand Down
7 changes: 2 additions & 5 deletions execution/executor-benchmark/src/transaction_committer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 2 additions & 8 deletions execution/executor-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,13 @@ pub trait BlockExecutorTrait: Send + Sync {
block_ids: Vec<HashValue>,
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<()>;

Expand Down
18 changes: 4 additions & 14 deletions execution/executor/src/block_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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())
Expand Down
4 changes: 1 addition & 3 deletions execution/executor/src/tests/chunk_executor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions execution/executor/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 78b3331

Please sign in to comment.