Skip to content
Closed
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
16 changes: 13 additions & 3 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,14 @@ pub struct ArtifactCollection {
pub enum BenchmarkRequestStatus {
WaitingForArtifacts,
ArtifactsReady,
InProgress,
InProgress {
/// Derived from looking at the last 30days of benchmarks which have
/// completed with the same profile, backend combination as this request
/// then adding that offset to the `started_at`. It's done this way as
/// we don't have (or really need) a `started_at` for a
/// `benchmark_request`
estimated_completed_at: Option<DateTime<Utc>>,
},
Completed {
completed_at: DateTime<Utc>,
duration: Duration,
Expand All @@ -824,7 +831,7 @@ impl BenchmarkRequestStatus {
match self {
Self::WaitingForArtifacts => BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR,
Self::ArtifactsReady => BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR,
Self::InProgress => BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR,
Self::InProgress { .. } => BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR,
Self::Completed { .. } => BENCHMARK_REQUEST_STATUS_COMPLETED_STR,
}
}
Expand All @@ -833,11 +840,14 @@ impl BenchmarkRequestStatus {
text: &str,
completion_date: Option<DateTime<Utc>>,
duration_ms: Option<i32>,
estimated_completed_at: Option<DateTime<Utc>>,
) -> anyhow::Result<Self> {
match text {
BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR => Ok(Self::WaitingForArtifacts),
BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR => Ok(Self::ArtifactsReady),
BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR => Ok(Self::InProgress),
BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR => Ok(Self::InProgress {
estimated_completed_at,
}),
BENCHMARK_REQUEST_STATUS_COMPLETED_STR => Ok(Self::Completed {
completed_at: completion_date.ok_or_else(|| {
anyhow!("No completion date for a completed BenchmarkRequestStatus")
Expand Down
19 changes: 9 additions & 10 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::selector::CompileTestCase;
use crate::{
ArtifactCollection, ArtifactId, ArtifactIdNumber, BenchmarkJob, BenchmarkJobConclusion,
BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestWithErrors,
BenchmarkSet, CodegenBackend, CollectorConfig, CompileBenchmark, Target,
BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestWithErrors, BenchmarkSet,
CodegenBackend, CollectorConfig, CompileBenchmark, Target,
};
use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step};
use chrono::{DateTime, Utc};
Expand Down Expand Up @@ -202,13 +202,9 @@ pub trait Connection: Send + Sync {
/// been completed yet. Pending statuses are `ArtifactsReady` and `InProgress`.
async fn load_pending_benchmark_requests(&self) -> anyhow::Result<Vec<BenchmarkRequest>>;

/// Update the status of a `benchmark_request` with the given `tag`.
/// Update the status of a `benchmark_request` with the given `tag` to `in_progress`
/// If no such request exists in the DB, returns an error.
async fn update_benchmark_request_status(
&self,
tag: &str,
status: BenchmarkRequestStatus,
) -> anyhow::Result<()>;
async fn set_benchmark_request_in_progress(&self, tag: &str) -> anyhow::Result<()>;

/// Update a Try commit to have a `sha` and `parent_sha`. Will update the
/// status of the request too a ready state.
Expand Down Expand Up @@ -424,6 +420,7 @@ mod tests {
use crate::metric::Metric;
use crate::tests::builder::{job, RequestBuilder};
use crate::tests::run_postgres_test;
use crate::BenchmarkRequestStatus;
use crate::{tests::run_db_test, BenchmarkRequestType, Commit, CommitType, Date};
use chrono::Utc;
use std::str::FromStr;
Expand Down Expand Up @@ -633,6 +630,7 @@ mod tests {
let target = Target::X86_64UnknownLinuxGnu;
let collector_name = "collector-1";
let benchmark_set = 1;
let tag = "1.79.0";

db.add_collector_config(collector_name, target, benchmark_set, true)
.await
Expand All @@ -653,11 +651,12 @@ mod tests {
db.insert_benchmark_request(req).await.unwrap();
}

complete_request(db, "1.79.0", collector_name, benchmark_set, target).await;
complete_request(db, tag, collector_name, benchmark_set, target).await;

db.update_benchmark_request_status("sha-2", BenchmarkRequestStatus::InProgress)
db.enqueue_benchmark_job(tag, target, CodegenBackend::Llvm, Profile::Opt, 0)
.await
.unwrap();
db.set_benchmark_request_in_progress("sha-2").await.unwrap();

let requests = db.load_pending_benchmark_requests().await.unwrap();

Expand Down
109 changes: 74 additions & 35 deletions database/src/pool/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ pub struct CachedStatements {
get_compile_test_cases_with_measurements: Statement,
get_last_n_completed_requests_with_errors: Statement,
get_jobs_of_in_progress_benchmark_requests: Statement,
get_pending_benchmark_requests: Statement,
}

pub struct PostgresTransaction<'a> {
Expand Down Expand Up @@ -748,6 +749,54 @@ impl PostgresConnection {
-- Only get requests that have some jobs
RIGHT JOIN job_queue on job_queue.request_tag = requests.tag
")).await.unwrap(),
// Finds a start time for the in_progress requests and estimates
// a completion time
get_pending_benchmark_requests: conn.prepare(&format!( "
WITH in_progress AS (
SELECT
{BENCHMARK_REQUEST_COLUMNS}
FROM
benchmark_request
WHERE
status = '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}' OR
status = '{BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR}'
), average_completion_time AS (
-- Find similar benchmark requests completion times
SELECT
AVG(benchmark_request.duration_ms) AS average_duration
FROM
benchmark_request
LEFT JOIN in_progress ON benchmark_request.backends = in_progress.backends
AND benchmark_request.profiles = in_progress.profiles
WHERE benchmark_request.status = '{BENCHMARK_REQUEST_STATUS_COMPLETED_STR}'
AND benchmark_request.completed_at > current_date - INTERVAL '30' DAY
LIMIT 30
), in_progress_started_at AS (
SELECT
tag AS request_tag,
started_at
FROM
benchmark_request
LEFT JOIN
job_queue ON benchmark_request.tag = job_queue.request_tag
WHERE
benchmark_request.status = '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}' ORDER BY started_at ASC LIMIT 1
)
SELECT
{BENCHMARK_REQUEST_COLUMNS},
CASE
WHEN status = '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}'
THEN COALESCE(started_at + (average_duration * INTERVAL '1 ms'), NOW() + INTERVAL '1 h')
ELSE NULL
END
AS estimated_completed_at
FROM
in_progress
LEFT JOIN
in_progress_started_at ON in_progress.tag = in_progress_started_at.request_tag
CROSS JOIN average_completion_time
ORDER BY status DESC
")).await.unwrap()
}),
conn,
}
Expand Down Expand Up @@ -1634,26 +1683,15 @@ where
Ok(BenchmarkRequestIndex { all, completed })
}

async fn update_benchmark_request_status(
&self,
tag: &str,
status: BenchmarkRequestStatus,
) -> anyhow::Result<()> {
// We cannot use this function to mark requests as complete, as
// we need to know if all jobs are complete first.
if matches!(status, BenchmarkRequestStatus::Completed { .. }) {
panic!("Please use `mark_benchmark_request_as_completed(...)` to complete benchmark_requests");
}

let status_str = status.as_str();
async fn set_benchmark_request_in_progress(&self, tag: &str) -> anyhow::Result<()> {
let modified_rows = self
.conn()
.execute(
r#"
UPDATE benchmark_request
SET status = $1
WHERE tag = $2;"#,
&[&status_str, &tag],
&[&BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR, &tag],
)
.await
.context("failed to update benchmark request status")?;
Expand Down Expand Up @@ -1703,24 +1741,17 @@ where
}

async fn load_pending_benchmark_requests(&self) -> anyhow::Result<Vec<BenchmarkRequest>> {
let query = format!(
r#"
SELECT {BENCHMARK_REQUEST_COLUMNS}
FROM benchmark_request
WHERE status IN('{BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR}', '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}')"#
);

let rows = self
Ok(self
.conn()
.query(&query, &[])
.query(&self.statements().get_pending_benchmark_requests, &[])
.await
.context("Failed to get pending benchmark requests")?;

let requests = rows
.context("Failed to get pending benchmark requests")?
.into_iter()
.map(|it| row_to_benchmark_request(&it, None))
.collect();
Ok(requests)
.map(|it| {
let estimated_completed_at = it.get::<_, Option<DateTime<Utc>>>(11);
row_to_benchmark_request(&it, None, estimated_completed_at)
})
.collect())
}

async fn enqueue_benchmark_job(
Expand Down Expand Up @@ -2110,7 +2141,7 @@ where
}
} else {
// We see this request for the first time
let request = row_to_benchmark_request(&row, None);
let request = row_to_benchmark_request(&row, None, None);
let request_errors = if let Some(benchmark) = error_benchmark {
HashMap::from([(benchmark, error_content.unwrap_or_default())])
} else {
Expand Down Expand Up @@ -2237,7 +2268,11 @@ where
}
}

fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRequest {
fn row_to_benchmark_request(
row: &Row,
row_offset: Option<usize>,
estimated_completed_at: Option<DateTime<Utc>>,
) -> BenchmarkRequest {
let row_offset = row_offset.unwrap_or(0);
let tag = row.get::<_, Option<String>>(row_offset);
let parent_sha = row.get::<_, Option<String>>(1 + row_offset);
Expand All @@ -2253,11 +2288,15 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe

let pr = pr.map(|v| v as u32);

let status =
BenchmarkRequestStatus::from_str_and_completion_date(status, completed_at, duration_ms)
.unwrap_or_else(|e| {
panic!("Invalid BenchmarkRequestStatus data in the database for tag {tag:?}: {e:?}")
});
let status = BenchmarkRequestStatus::from_str_and_completion_date(
status,
completed_at,
duration_ms,
estimated_completed_at,
)
.unwrap_or_else(|e| {
panic!("Invalid BenchmarkRequestStatus data in the database for tag {tag:?}: {e:?}")
});

match commit_type {
BENCHMARK_REQUEST_TRY_STR => BenchmarkRequest {
Expand Down
12 changes: 4 additions & 8 deletions database/src/pool/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction}
use crate::selector::CompileTestCase;
use crate::{
ArtifactCollection, ArtifactId, Benchmark, BenchmarkJob, BenchmarkJobConclusion,
BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestWithErrors,
BenchmarkSet, CodegenBackend, CollectionId, CollectorConfig, Commit, CommitType,
CompileBenchmark, Date, Profile, Target,
BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestWithErrors, BenchmarkSet,
CodegenBackend, CollectionId, CollectorConfig, Commit, CommitType, CompileBenchmark, Date,
Profile, Target,
};
use crate::{ArtifactIdNumber, Index, QueuedCommit};
use chrono::{DateTime, TimeZone, Utc};
Expand Down Expand Up @@ -1310,11 +1310,7 @@ impl Connection for SqliteConnection {
no_queue_implementation_abort!()
}

async fn update_benchmark_request_status(
&self,
_tag: &str,
_status: BenchmarkRequestStatus,
) -> anyhow::Result<()> {
async fn set_benchmark_request_in_progress(&self, _tag: &str) -> anyhow::Result<()> {
no_queue_implementation_abort!()
}

Expand Down
6 changes: 3 additions & 3 deletions database/src/tests/builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
BenchmarkJob, BenchmarkJobConclusion, BenchmarkRequest, BenchmarkRequestStatus, BenchmarkSet,
CodegenBackend, CollectorConfig, Connection, Profile, Target,
BenchmarkJob, BenchmarkJobConclusion, BenchmarkRequest, BenchmarkSet, CodegenBackend,
CollectorConfig, Connection, Profile, Target,
};
use chrono::Utc;
use hashbrown::{HashMap, HashSet};
Expand Down Expand Up @@ -55,7 +55,7 @@ impl RequestBuilder {
}

pub async fn set_in_progress(self, db: &dyn Connection) -> Self {
db.update_benchmark_request_status(self.tag(), BenchmarkRequestStatus::InProgress)
db.set_benchmark_request_in_progress(self.tag())
.await
.unwrap();
self
Expand Down
1 change: 1 addition & 0 deletions site/frontend/src/pages/status_new/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type BenchmarkJob = {
completedAt: string | null;
status: BenchmarkJobStatus;
dequeCounter: number;
estimatedCompletedAt: string | null;
};

export type CollectorConfig = {
Expand Down
9 changes: 8 additions & 1 deletion site/frontend/src/pages/status_new/page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ function PullRequestLink({request}: {request: BenchmarkRequest}) {
);
}

function getCompletedAt(req: BenchmarkRequest) {
if (req.status === "Completed") {
return formatISODate(req.completedAt);
}
return `<strong>${formatISODate(req.completedAt)} (est.)</strong>`;
}

const {toggleExpanded: toggleExpandedErrors, isExpanded: hasExpandedErrors} =
useExpandedStore();

Expand Down Expand Up @@ -158,7 +165,7 @@ loadStatusData(loading);
req.status === "Completed" && req.hasPendingJobs ? "*" : ""
}}
</td>
<td v-html="formatISODate(req.completedAt)"></td>
<td v-html="getCompletedAt(req)"></td>
<td v-html="getDuration(req)"></td>

<td v-if="hasErrors(req.errors)">
Expand Down
1 change: 1 addition & 0 deletions site/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ pub mod status_new {
pub status: BenchmarkRequestStatus,
pub request_type: BenchmarkRequestType,
pub created_at: DateTime<Utc>,
pub estimated_completed_at: Option<DateTime<Utc>>,
pub completed_at: Option<DateTime<Utc>>,
pub duration_s: Option<u64>,
pub errors: HashMap<String, String>,
Expand Down
13 changes: 5 additions & 8 deletions site/src/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub async fn build_queue(

// The queue starts with in progress
let mut queue: Vec<BenchmarkRequest> = pending.extract_if_stable(|request| {
matches!(request.status(), BenchmarkRequestStatus::InProgress)
matches!(request.status(), BenchmarkRequestStatus::InProgress { .. })
});

// We sort the in-progress ones based on the started date
Expand Down Expand Up @@ -231,7 +231,7 @@ pub async fn enqueue_benchmark_request(
}

tx.conn()
.update_benchmark_request_status(request_tag, BenchmarkRequestStatus::InProgress)
.set_benchmark_request_in_progress(request_tag)
.await?;
tx.commit().await?;
Ok(())
Expand All @@ -252,7 +252,7 @@ async fn try_enqueue_next_benchmark_request(
enqueue_benchmark_request(conn, &request).await?;
break;
}
BenchmarkRequestStatus::InProgress => {
BenchmarkRequestStatus::InProgress { .. } => {
if conn
.maybe_mark_benchmark_request_as_completed(request.tag().unwrap())
.await?
Expand Down Expand Up @@ -309,8 +309,7 @@ mod tests {
use chrono::Utc;
use database::tests::run_postgres_test;
use database::{
BenchmarkJobConclusion, BenchmarkRequest, BenchmarkRequestStatus, BenchmarkSet,
CodegenBackend, Profile, Target,
BenchmarkJobConclusion, BenchmarkRequest, BenchmarkSet, CodegenBackend, Profile, Target,
};

fn create_master(sha: &str, parent: &str, pr: u32) -> BenchmarkRequest {
Expand Down Expand Up @@ -474,9 +473,7 @@ mod tests {
db.attach_shas_to_try_benchmark_request(16, "try1", "rrr", Utc::now())
.await
.unwrap();
db.update_benchmark_request_status("try1", BenchmarkRequestStatus::InProgress)
.await
.unwrap();
db.set_benchmark_request_in_progress("try1").await.unwrap();
db.attach_shas_to_try_benchmark_request(17, "baz", "foo", Utc::now())
.await
.unwrap();
Expand Down
Loading
Loading