Skip to content

Commit 4e8b6ed

Browse files
committed
[mfp] log actions during transaction submission (#23846)
## Description Log transaction submission steps with timestamps, for debugging and latency investigations. ## Test plan CI
1 parent b0a2e6c commit 4e8b6ed

File tree

9 files changed

+137
-76
lines changed

9 files changed

+137
-76
lines changed

crates/sui-core/src/authority_server.rs

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use anyhow::Result;
66
use async_trait::async_trait;
77
use fastcrypto::traits::KeyPair;
88
use futures::{future, TryFutureExt};
9+
use itertools::Itertools as _;
910
use mysten_metrics::spawn_monitored_task;
1011
use prometheus::{
1112
register_gauge_with_registry, register_histogram_vec_with_registry,
@@ -566,7 +567,12 @@ impl ValidatorService {
566567
Ok((tonic::Response::new(info), Weight::zero()))
567568
}
568569

569-
#[instrument(name= "ValidatorService::handle_submit_transaction", level = "error", skip_all, err(level = "debug"), fields(tx_digest = ?tracing::field::Empty))]
570+
#[instrument(
571+
name = "ValidatorService::handle_submit_transaction",
572+
level = "error",
573+
skip_all,
574+
err(level = "debug")
575+
)]
570576
async fn handle_submit_transaction(
571577
&self,
572578
request: tonic::Request<RawSubmitTxRequest>,
@@ -644,6 +650,8 @@ impl ValidatorService {
644650
.into()
645651
);
646652

653+
// Transaction digests.
654+
let mut tx_digests = Vec::with_capacity(request.transactions.len());
647655
// Transactions to submit to consensus.
648656
let mut consensus_transactions = Vec::with_capacity(request.transactions.len());
649657
// Indexes of transactions above in the request transactions.
@@ -714,11 +722,12 @@ impl ValidatorService {
714722
};
715723

716724
let tx_digest = verified_transaction.digest();
725+
tx_digests.push(*tx_digest);
717726

718-
// Only trace the 1st transaction in the request.
719-
if idx == 0 {
720-
tracing::Span::current().record("tx_digest", tracing::field::debug(&tx_digest));
721-
}
727+
debug!(
728+
?tx_digest,
729+
"handle_submit_transaction: verified transaction"
730+
);
722731

723732
// Check if the transaction has executed, before checking input objects
724733
// which could have been consumed.
@@ -744,17 +753,22 @@ impl ValidatorService {
744753
fast_path: false,
745754
};
746755
results[idx] = Some(executed_result);
756+
debug!(?tx_digest, "handle_submit_transaction: already executed");
747757
continue;
748758
}
749759
}
750760

761+
debug!(
762+
?tx_digest,
763+
"handle_submit_transaction: waiting for fastpath dependency objects"
764+
);
751765
if !state
752766
.wait_for_fastpath_dependency_objects(&verified_transaction, epoch_store.epoch())
753767
.await?
754768
{
755769
debug!(
756770
?tx_digest,
757-
"Fastpath input objects are still unavailable after waiting"
771+
"fastpath input objects are still unavailable after waiting"
758772
);
759773
}
760774

@@ -859,6 +873,14 @@ impl ValidatorService {
859873
is_ping_request || !consensus_transactions.is_empty(),
860874
"A valid soft bundle must have at least one transaction"
861875
);
876+
debug!(
877+
"handle_submit_transaction: submitting consensus transactions ({}): {}",
878+
req_type,
879+
consensus_transactions
880+
.iter()
881+
.map(|t| t.local_display())
882+
.join(", ")
883+
);
862884
self.handle_submit_to_consensus_for_position(
863885
consensus_transactions,
864886
&epoch_store,
@@ -867,6 +889,11 @@ impl ValidatorService {
867889
.await?
868890
} else {
869891
let futures = consensus_transactions.into_iter().map(|t| {
892+
debug!(
893+
"handle_submit_transaction: submitting consensus transaction ({}): {}",
894+
req_type,
895+
t.local_display(),
896+
);
870897
self.handle_submit_to_consensus_for_position(
871898
vec![t],
872899
&epoch_store,
@@ -888,9 +915,16 @@ impl ValidatorService {
888915
}));
889916
} else {
890917
// Otherwise, return the consensus position for each transaction.
891-
for (idx, consensus_position) in
892-
transaction_indexes.into_iter().zip(consensus_positions)
918+
for ((idx, tx_digest), consensus_position) in transaction_indexes
919+
.into_iter()
920+
.zip(tx_digests)
921+
.zip(consensus_positions)
893922
{
923+
debug!(
924+
?tx_digest,
925+
"handle_submit_transaction: submitted consensus transaction at {}",
926+
consensus_position,
927+
);
894928
results[idx] = Some(SubmitTxResult::Submitted { consensus_position });
895929
}
896930
}
@@ -965,6 +999,7 @@ impl ValidatorService {
965999
// When multiple certificates are provided, we will either submit all of them or none of them to consensus.
9661000
if certificates.len() == 1 {
9671001
let tx_digest = *certificates[0].digest();
1002+
debug!(tx_digest=?tx_digest, "Checking if certificate is already executed");
9681003

9691004
if let Some(signed_effects) = self
9701005
.state
@@ -1075,7 +1110,7 @@ impl ValidatorService {
10751110
name = "ValidatorService::handle_submit_to_consensus_for_position",
10761111
level = "debug",
10771112
skip_all,
1078-
err
1113+
err(level = "debug")
10791114
)]
10801115
async fn handle_submit_to_consensus_for_position(
10811116
&self,
@@ -1418,7 +1453,7 @@ impl ValidatorService {
14181453
request: WaitForEffectsRequest,
14191454
epoch_store: &Arc<AuthorityPerEpochStore>,
14201455
) -> SuiResult<WaitForEffectsResponse> {
1421-
if request.ping.is_some() {
1456+
if request.ping_type.is_some() {
14221457
return timeout(
14231458
Duration::from_secs(10),
14241459
self.ping_response(request, epoch_store),
@@ -1502,7 +1537,7 @@ impl ValidatorService {
15021537
};
15031538

15041539
// We assume that the caller has already checked for the existence of the `ping` field, but handling it gracefully here.
1505-
let Some(ping) = request.ping else {
1540+
let Some(ping) = request.ping_type else {
15061541
return Err(SuiError::InvalidRequest(
15071542
"Ping type is required for ping requests".to_string(),
15081543
));

crates/sui-core/src/transaction_driver/effects_certifier.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ impl EffectsCertifier {
7070
// Guaranteed to be not the Rejected variant.
7171
submit_txn_result: SubmitTxResult,
7272
options: &SubmitTransactionOptions,
73-
ping: Option<PingType>,
7473
) -> Result<QuorumTransactionResponse, TransactionDriverError>
7574
where
7675
A: AuthorityAPI + Send + Sync + 'static + Clone,
@@ -117,7 +116,6 @@ impl EffectsCertifier {
117116
consensus_position,
118117
options,
119118
current_target,
120-
ping
121119
),
122120
async {
123121
// No need to send a full effects request if it is already provided.
@@ -132,7 +130,7 @@ impl EffectsCertifier {
132130
.expect("there should be at least 1 target");
133131
current_target = name;
134132
full_effects_start_time = Some(Instant::now());
135-
self.get_full_effects(client, tx_digest, consensus_position, options, ping)
133+
self.get_full_effects(client, tx_digest, tx_type, consensus_position, options)
136134
.await
137135
},
138136
);
@@ -200,7 +198,7 @@ impl EffectsCertifier {
200198
current_target = name;
201199
full_effects_start_time = Some(Instant::now());
202200
full_effects_result = self
203-
.get_full_effects(client, tx_digest, consensus_position, options, ping)
201+
.get_full_effects(client, tx_digest, tx_type, consensus_position, options)
204202
.await;
205203
}
206204
}
@@ -210,18 +208,19 @@ impl EffectsCertifier {
210208
&self,
211209
client: Arc<SafeClient<A>>,
212210
tx_digest: Option<TransactionDigest>,
211+
tx_type: TxType,
213212
consensus_position: Option<ConsensusPosition>,
214213
options: &SubmitTransactionOptions,
215-
ping: Option<PingType>,
216214
) -> Result<(TransactionEffectsDigest, Box<ExecutedData>, bool), TransactionRequestError>
217215
where
218216
A: AuthorityAPI + Send + Sync + 'static + Clone,
219217
{
218+
let ping_type = get_ping_type(&tx_digest, tx_type);
220219
let request = WaitForEffectsRequest {
221220
transaction_digest: tx_digest,
222221
consensus_position,
223222
include_details: true,
224-
ping,
223+
ping_type,
225224
};
226225

227226
match timeout(
@@ -272,12 +271,12 @@ impl EffectsCertifier {
272271
consensus_position: Option<ConsensusPosition>,
273272
options: &SubmitTransactionOptions,
274273
submitted_tx_to_validator: AuthorityName,
275-
ping: Option<PingType>,
276274
) -> Result<TransactionEffectsDigest, TransactionDriverError>
277275
where
278276
A: AuthorityAPI + Send + Sync + 'static + Clone,
279277
{
280-
let ping_label = if ping.is_some() { "true" } else { "false" };
278+
let ping_type = get_ping_type(&tx_digest, tx_type);
279+
let ping_label = if tx_digest.is_none() { "true" } else { "false" };
281280
self.metrics
282281
.certified_effects_ack_attempts
283282
.with_label_values(&[tx_type.as_str(), ping_label])
@@ -292,7 +291,7 @@ impl EffectsCertifier {
292291
transaction_digest: tx_digest,
293292
consensus_position,
294293
include_details: false,
295-
ping,
294+
ping_type,
296295
})
297296
.unwrap();
298297

@@ -650,3 +649,14 @@ impl EffectsCertifier {
650649
}
651650
}
652651
}
652+
653+
fn get_ping_type(tx_digest: &Option<TransactionDigest>, tx_type: TxType) -> Option<PingType> {
654+
if tx_digest.is_none() {
655+
Some(match tx_type {
656+
TxType::SingleWriter => PingType::FastPath,
657+
TxType::SharedObject => PingType::Consensus,
658+
})
659+
} else {
660+
None
661+
}
662+
}

crates/sui-core/src/transaction_driver/mod.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ where
178178
}
179179
let start_time = Instant::now();
180180

181-
let ping = if tx_type == TxType::SingleWriter {
181+
let ping_type = if tx_type == TxType::SingleWriter {
182182
PingType::FastPath
183183
} else {
184184
PingType::Consensus
@@ -187,7 +187,7 @@ where
187187
// Now send a ping transaction to the chosen validator for the provided tx type
188188
match self_clone
189189
.drive_transaction(
190-
SubmitTxRequest::new_ping(ping),
190+
SubmitTxRequest::new_ping(ping_type),
191191
SubmitTransactionOptions {
192192
allowed_validators: vec![name],
193193
..Default::default()
@@ -214,16 +214,16 @@ where
214214
}
215215
}
216216

217-
/// Drives transaction to submission and effects certification. If ping is provided, then the requested will be treated as a ping transaction.
218-
#[instrument(level = "error", skip_all, fields(tx_digest = ?request.transaction.as_ref().map(|t| t.digest()), ping = ?request.ping))]
217+
/// Drives transaction to submission and effects certification. Ping requests are derived from the submitted payload.
218+
#[instrument(level = "error", skip_all, fields(tx_digest = ?request.transaction.as_ref().map(|t| t.digest()), ping = %request.ping_type.is_some()))]
219219
pub async fn drive_transaction(
220220
&self,
221221
request: SubmitTxRequest,
222222
options: SubmitTransactionOptions,
223223
timeout_duration: Option<Duration>,
224224
) -> Result<QuorumTransactionResponse, TransactionDriverError> {
225225
// For ping requests, the amplification factor is always 1.
226-
let amplification_factor = if request.ping.is_some() {
226+
let amplification_factor = if request.ping_type.is_some() {
227227
1
228228
} else {
229229
let gas_price = request
@@ -247,7 +247,7 @@ where
247247
};
248248

249249
let tx_type = request.tx_type();
250-
let ping_label = if request.ping.is_some() {
250+
let ping_label = if request.ping_type.is_some() {
251251
"true"
252252
} else {
253253
"false"
@@ -358,7 +358,6 @@ where
358358
let amplification_factor =
359359
amplification_factor.min(auth_agg.committee.num_members() as u64);
360360
let start_time = Instant::now();
361-
let ping = request.ping;
362361
let tx_type = request.tx_type();
363362
let tx_digest = request.tx_digest();
364363

@@ -390,7 +389,6 @@ where
390389
name,
391390
submit_txn_result,
392391
options,
393-
ping,
394392
)
395393
.await;
396394

crates/sui-core/src/transaction_driver/transaction_submitter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl TransactionSubmitter {
7373
options.allowed_validators.clone(),
7474
);
7575

76-
let ping_label = if request.ping.is_some() {
76+
let ping_label = if request.ping_type.is_some() {
7777
"true"
7878
} else {
7979
"false"

0 commit comments

Comments
 (0)