Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ impl BenchmarkRequest {
}

pub fn is_in_progress(&self) -> bool {
matches!(self.status, BenchmarkRequestStatus::InProgress { .. })
matches!(self.status, BenchmarkRequestStatus::InProgress)
}
}

Expand Down
34 changes: 34 additions & 0 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,18 @@ pub trait Connection: Send + Sync {
benchmark_set: u32,
) -> anyhow::Result<u32>;

/// Add a benchmark job which is explicitly using a `parent_sha` we split
/// this out to improve our error handling. A `parent_sha` may not have
/// an associated request in the `benchmarek`
async fn enqueue_parent_benchmark_job(
&self,
parent_sha: &str,
target: Target,
backend: CodegenBackend,
profile: Profile,
benchmark_set: u32,
) -> (bool, anyhow::Result<u32>);

/// Returns a set of compile-time benchmark test cases that were already computed for the
/// given artifact.
/// Note that for efficiency reasons, the function only checks if we have at least a single
Expand Down Expand Up @@ -1187,4 +1199,26 @@ mod tests {
})
.await;
}

#[tokio::test]
async fn enqueue_parent_benchmark_job() {
run_postgres_test(|ctx| async {
let db = ctx.db();

let (violates_foreign_key, _) = db
.enqueue_parent_benchmark_job(
"sha-0",
Target::X86_64UnknownLinuxGnu,
CodegenBackend::Llvm,
Profile::Debug,
0,
)
.await;

assert!(violates_foreign_key);

Ok(ctx)
})
.await;
}
}
59 changes: 59 additions & 0 deletions database/src/pool/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use std::str::FromStr;
use std::sync::Arc;
use std::time::Duration;
use tokio::sync::Mutex;
use tokio_postgres::error::SqlState;
use tokio_postgres::Statement;
use tokio_postgres::{GenericClient, Row};

Expand Down Expand Up @@ -1738,6 +1739,64 @@ where
})
}

async fn enqueue_parent_benchmark_job(
&self,
parent_sha: &str,
target: Target,
backend: CodegenBackend,
profile: Profile,
benchmark_set: u32,
) -> (bool, anyhow::Result<u32>) {
let row_result = self
.conn()
.query_one(
r#"
INSERT INTO job_queue(
request_tag,
target,
backend,
profile,
benchmark_set,
status
)
VALUES ($1, $2, $3, $4, $5, $6)
ON CONFLICT DO NOTHING
RETURNING job_queue.id
"#,
&[
&parent_sha,
&target,
&backend,
&profile,
&(benchmark_set as i32),
&BENCHMARK_JOB_STATUS_QUEUED_STR,
],
)
.await;

match row_result {
Ok(row) => (false, Ok(row.get::<_, i32>(0) as u32)),
Err(e) => {
if let Some(db_err) = e.as_db_error() {
if db_err.code() == &SqlState::FOREIGN_KEY_VIOLATION {
let constraint = db_err.constraint().unwrap_or("benchmark_tag constraint");
let detail = db_err.detail().unwrap_or("");
return (
true,
Err(anyhow::anyhow!(
"Foreign key violation on {} for request_tag='{}'. {}",
constraint,
parent_sha,
detail
)),
);
}
}
(false, Err(e.into()))
}
}
}

async fn enqueue_benchmark_job(
&self,
request_tag: &str,
Expand Down
11 changes: 11 additions & 0 deletions database/src/pool/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,17 @@ impl Connection for SqliteConnection {
no_queue_implementation_abort!()
}

async fn enqueue_parent_benchmark_job(
&self,
_parent_sha: &str,
_target: Target,
_backend: CodegenBackend,
_profile: Profile,
_benchmark_set: u32,
) -> (bool, anyhow::Result<u32>) {
no_queue_implementation_abort!()
}

async fn get_compile_test_cases_with_measurements(
&self,
artifact_row_id: &ArtifactIdNumber,
Expand Down
23 changes: 20 additions & 3 deletions site/src/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ pub async fn enqueue_benchmark_request(

let backends = benchmark_request.backends()?;
let profiles = benchmark_request.profiles()?;
// Prevent the error from spamming the logs
let mut has_emitted_parent_sha_error = false;

// Target x benchmark_set x backend x profile -> BenchmarkJob
for target in Target::all() {
Expand All @@ -237,15 +239,30 @@ pub async fn enqueue_benchmark_request(
// but was already benchmarked then the collector will ignore
// it as it will see it already has results.
if let Some(parent_sha) = benchmark_request.parent_sha() {
tx.conn()
.enqueue_benchmark_job(
let (is_foreign_key_violation, result) = tx
.conn()
.enqueue_parent_benchmark_job(
parent_sha,
target,
backend,
profile,
benchmark_set as u32,
)
.await?;
.await;

// At some point in time the parent_sha may not refer
// to a `benchmark_request` and we want to be able to
// see that error.
if let Err(e) = result {
if is_foreign_key_violation && !has_emitted_parent_sha_error {
log::error!("Failed to create job for parent sha {e:?}");
has_emitted_parent_sha_error = true;
} else if has_emitted_parent_sha_error && is_foreign_key_violation {
continue;
} else {
return Err(e);
}
}
}
}
}
Expand Down
Loading