Skip to content

Commit 392c285

Browse files
committed
control-plane-api: log publication commit timing
Adds to our publication commit logging so that we can get some more performance information.
1 parent f5095c2 commit 392c285

File tree

1 file changed

+56
-10
lines changed
  • crates/control-plane-api/src/publications

1 file changed

+56
-10
lines changed

crates/control-plane-api/src/publications/mod.rs

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::u32;
2+
13
use super::logs;
24
use anyhow::Context;
35
use chrono::{DateTime, Utc};
@@ -500,6 +502,7 @@ impl Publisher {
500502
!uncommitted.has_errors(),
501503
"cannot commit uncommitted build that has errors"
502504
);
505+
let start_time = std::time::Instant::now();
503506

504507
// Assign live spec ids prior to attempting commit. This simplifies the
505508
// commit process, which would otherwise need to determine the IDs and
@@ -511,33 +514,72 @@ impl Publisher {
511514
// return a lock failure after retrying here. Lock failures can also be
512515
// retried, but doing so requires a fresh build, and so is handled
513516
// outside of this function.
514-
for attempt in 0..10 {
517+
const MAX_ATTEMPTS: usize = 10;
518+
for attempt in 0..MAX_ATTEMPTS {
515519
let completed_at = Utc::now();
516-
match self.try_commit(&uncommitted, &with_commit).await {
520+
let attempt_start_time = std::time::Instant::now();
521+
let commit_result = self.try_commit(&uncommitted, &with_commit).await;
522+
523+
let attempt_elapsed_ms = elapsed_ms(attempt_start_time);
524+
let commit_elapsed_ms = elapsed_ms(start_time);
525+
526+
match commit_result {
517527
Ok((_, quota_errors)) if !quota_errors.is_empty() => {
528+
tracing::info!(
529+
attempt,
530+
attempt_elapsed_ms,
531+
commit_elapsed_ms,
532+
result = "overQuotaError",
533+
"finished publication commit"
534+
);
518535
let mut result =
519536
uncommitted.into_result(completed_at, StatusType::PublishFailed.into());
520537
result.built.errors.extend(quota_errors.into_iter());
521538
return Ok(result);
522539
}
523540
Ok((lock_failures, _)) if !lock_failures.is_empty() => {
541+
tracing::info!(
542+
attempt,
543+
attempt_elapsed_ms,
544+
commit_elapsed_ms,
545+
result = "buildIdLockFailure",
546+
"finished publication commit"
547+
);
524548
return Ok(uncommitted.into_result(
525549
completed_at,
526550
JobStatus::build_id_lock_failure(lock_failures),
527551
));
528552
}
529553
Ok(_no_failures) => {
530-
tracing::info!("successfully committed publication");
554+
tracing::info!(
555+
attempt,
556+
attempt_elapsed_ms,
557+
commit_elapsed_ms,
558+
result = "success",
559+
// TODO(phil): remove after dashboards are updated
560+
temp_dashboard_compat = "successfully committed publication",
561+
"finished publication commit"
562+
);
531563
return Ok(uncommitted.into_result(completed_at, StatusType::Success.into()));
532564
}
533565
Err(err) if is_transaction_serialization_error(&err) => {
534-
let jitter = rand::rng().random_range(0..500);
535-
tracing::debug!(
536-
attempt,
537-
backoff_ms = jitter,
538-
"retrying commit due to transaction serialization failure"
539-
);
540-
tokio::time::sleep(std::time::Duration::from_millis(jitter)).await;
566+
if attempt == (MAX_ATTEMPTS - 1) {
567+
tracing::info!(
568+
attempt,
569+
attempt_elapsed_ms,
570+
commit_elapsed_ms,
571+
result = "optimisticLockingFailureLimit",
572+
"finished publication commit"
573+
);
574+
} else {
575+
let jitter = rand::rng().random_range(0..500);
576+
tracing::debug!(
577+
attempt,
578+
backoff_ms = jitter,
579+
"retrying commit due to transaction serialization failure"
580+
);
581+
tokio::time::sleep(std::time::Duration::from_millis(jitter)).await;
582+
}
541583
}
542584
Err(err) => return Err(err),
543585
}
@@ -664,6 +706,10 @@ fn is_empty_draft(build: &UncommittedBuild) -> bool {
664706
&& built.built_tests.iter().all(BuiltRow::is_passthrough)
665707
}
666708

709+
fn elapsed_ms(from: std::time::Instant) -> u32 {
710+
u32::try_from(from.elapsed().as_millis()).unwrap_or(u32::MAX)
711+
}
712+
667713
/// Determines whether the given error is due to a transaction serialization failure,
668714
/// meaning that the commit must be retried.
669715
fn is_transaction_serialization_error(err: &anyhow::Error) -> bool {

0 commit comments

Comments
 (0)