Skip to content

Commit

Permalink
Cleanup: use instrument to log checkpoint sequence number in checkpoi…
Browse files Browse the repository at this point in the history
…nt execution (MystenLabs#15154)

## Description 

To remove redundant `checkpoint_seq=?checkpoint.sequence_number()` in
all the nested debugging logging.

## Test Plan 

How did you test the new or updated feature?

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
halfprice authored Dec 6, 2023
1 parent dd6a8d9 commit b7074d0
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions crates/sui-core/src/checkpoints/checkpoint_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ impl CheckpointExecutor {
pending: &mut CheckpointExecutionBuffer,
epoch_store: Arc<AuthorityPerEpochStore>,
) {
debug!(checkpoint_seq=?checkpoint.sequence_number(), "Scheduling checkpoint for execution");
debug!("Scheduling checkpoint for execution");

// Mismatch between node epoch and checkpoint epoch after startup
// crash recovery is invalid
Expand Down Expand Up @@ -516,7 +516,7 @@ impl CheckpointExecutor {

// Logs within the function are annotated with the checkpoint sequence number and epoch,
// from schedule_checkpoint().
#[instrument(level = "debug", skip_all)]
#[instrument(level = "debug", skip_all, fields(seq = ?checkpoint.sequence_number(), epoch = ?epoch_store.epoch()))]
async fn execute_checkpoint(
checkpoint: VerifiedCheckpoint,
authority_store: Arc<AuthorityStore>,
Expand All @@ -528,7 +528,7 @@ async fn execute_checkpoint(
metrics: &Arc<CheckpointExecutorMetrics>,
data_ingestion_dir: Option<PathBuf>,
) -> SuiResult {
debug!(checkpoint_seq=?checkpoint.sequence_number(), "Preparing checkpoint for execution",);
debug!("Preparing checkpoint for execution",);
let prepare_start = Instant::now();

// this function must guarantee that all transactions in the checkpoint are executed before it
Expand All @@ -545,7 +545,7 @@ async fn execute_checkpoint(
);

let tx_count = execution_digests.len();
debug!(checkpoint_seq=?checkpoint.sequence_number(), "Number of transactions in the checkpoint: {:?}", tx_count);
debug!("Number of transactions in the checkpoint: {:?}", tx_count);
metrics.checkpoint_transaction_count.report(tx_count as u64);

execute_transactions(
Expand Down Expand Up @@ -983,7 +983,6 @@ async fn execute_transactions(
.report(prepare_elapsed.as_micros() as u64);
if checkpoint.sequence_number % CHECKPOINT_PROGRESS_LOG_COUNT_INTERVAL == 0 {
info!(
checkpoint_seq=?checkpoint.sequence_number(),
"Checkpoint preparation for execution took {:?}",
prepare_elapsed
);
Expand Down Expand Up @@ -1012,7 +1011,7 @@ async fn execute_transactions(
.checkpoint_exec_latency_us
.report(exec_elapsed.as_micros() as u64);
if checkpoint.sequence_number % CHECKPOINT_PROGRESS_LOG_COUNT_INTERVAL == 0 {
info!(checkpoint_seq = ?checkpoint.sequence_number(), "Checkpoint execution took {:?}", exec_elapsed);
info!("Checkpoint execution took {:?}", exec_elapsed);
}

Ok(())
Expand Down

0 comments on commit b7074d0

Please sign in to comment.