diff --git a/database/src/lib.rs b/database/src/lib.rs index 9633044bc..2de4aa4f7 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -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>, + }, Completed { completed_at: DateTime, duration: Duration, @@ -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, } } @@ -833,11 +840,14 @@ impl BenchmarkRequestStatus { text: &str, completion_date: Option>, duration_ms: Option, + estimated_completed_at: Option>, ) -> anyhow::Result { 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") diff --git a/database/src/pool.rs b/database/src/pool.rs index 9ffc3b89a..14180403f 100644 --- a/database/src/pool.rs +++ b/database/src/pool.rs @@ -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}; @@ -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>; - /// 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. @@ -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; @@ -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 @@ -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(); diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index ce764a617..f00405c3c 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -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> { @@ -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, } @@ -1634,18 +1683,7 @@ 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( @@ -1653,7 +1691,7 @@ where 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")?; @@ -1703,24 +1741,17 @@ where } async fn load_pending_benchmark_requests(&self) -> anyhow::Result> { - 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>>(11); + row_to_benchmark_request(&it, None, estimated_completed_at) + }) + .collect()) } async fn enqueue_benchmark_job( @@ -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 { @@ -2237,7 +2268,11 @@ where } } -fn row_to_benchmark_request(row: &Row, row_offset: Option) -> BenchmarkRequest { +fn row_to_benchmark_request( + row: &Row, + row_offset: Option, + estimated_completed_at: Option>, +) -> BenchmarkRequest { let row_offset = row_offset.unwrap_or(0); let tag = row.get::<_, Option>(row_offset); let parent_sha = row.get::<_, Option>(1 + row_offset); @@ -2253,11 +2288,15 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option) -> 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 { diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index f36c77f52..6530b3533 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -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}; @@ -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!() } diff --git a/database/src/tests/builder.rs b/database/src/tests/builder.rs index 48679e657..889c2569f 100644 --- a/database/src/tests/builder.rs +++ b/database/src/tests/builder.rs @@ -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}; @@ -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 diff --git a/site/frontend/src/pages/status_new/data.ts b/site/frontend/src/pages/status_new/data.ts index 27b9aa81b..098733d2e 100644 --- a/site/frontend/src/pages/status_new/data.ts +++ b/site/frontend/src/pages/status_new/data.ts @@ -25,6 +25,7 @@ export type BenchmarkJob = { completedAt: string | null; status: BenchmarkJobStatus; dequeCounter: number; + estimatedCompletedAt: string | null; }; export type CollectorConfig = { diff --git a/site/frontend/src/pages/status_new/page.vue b/site/frontend/src/pages/status_new/page.vue index a1c3ed4b6..3d787efed 100644 --- a/site/frontend/src/pages/status_new/page.vue +++ b/site/frontend/src/pages/status_new/page.vue @@ -118,6 +118,13 @@ function PullRequestLink({request}: {request: BenchmarkRequest}) { ); } +function getCompletedAt(req: BenchmarkRequest) { + if (req.status === "Completed") { + return formatISODate(req.completedAt); + } + return `${formatISODate(req.completedAt)} (est.)`; +} + const {toggleExpanded: toggleExpandedErrors, isExpanded: hasExpandedErrors} = useExpandedStore(); @@ -158,7 +165,7 @@ loadStatusData(loading); req.status === "Completed" && req.hasPendingJobs ? "*" : "" }} - + diff --git a/site/src/api.rs b/site/src/api.rs index c18d0ab4f..ed91e0e05 100644 --- a/site/src/api.rs +++ b/site/src/api.rs @@ -418,6 +418,7 @@ pub mod status_new { pub status: BenchmarkRequestStatus, pub request_type: BenchmarkRequestType, pub created_at: DateTime, + pub estimated_completed_at: Option>, pub completed_at: Option>, pub duration_s: Option, pub errors: HashMap, diff --git a/site/src/job_queue/mod.rs b/site/src/job_queue/mod.rs index 3969db96d..df278faae 100644 --- a/site/src/job_queue/mod.rs +++ b/site/src/job_queue/mod.rs @@ -155,7 +155,7 @@ pub async fn build_queue( // The queue starts with in progress let mut queue: Vec = 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 @@ -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(()) @@ -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? @@ -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 { @@ -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(); diff --git a/site/src/request_handlers/status_page_new.rs b/site/src/request_handlers/status_page_new.rs index f14def2cc..14f77e4b0 100644 --- a/site/src/request_handlers/status_page_new.rs +++ b/site/src/request_handlers/status_page_new.rs @@ -121,22 +121,27 @@ fn request_to_ui( req: &BenchmarkRequest, errors: HashMap, ) -> status_new::BenchmarkRequest { - let (completed_at, duration_s) = match req.status() { - BenchmarkRequestStatus::WaitingForArtifacts => (None, None), - BenchmarkRequestStatus::ArtifactsReady => (None, None), - BenchmarkRequestStatus::InProgress => (None, None), + let (completed_at, duration_s, estimated_completed_at) = match req.status() { + BenchmarkRequestStatus::WaitingForArtifacts => (None, None, None), + BenchmarkRequestStatus::ArtifactsReady => (None, None, None), + BenchmarkRequestStatus::InProgress { + estimated_completed_at, + } => (None, None, estimated_completed_at), BenchmarkRequestStatus::Completed { completed_at, duration, - } => (Some(completed_at), Some(duration.as_secs())), + } => (Some(completed_at), Some(duration.as_secs()), None), }; + status_new::BenchmarkRequest { tag: req.tag().expect("Missing request tag").to_string(), pr: req.pr(), status: match req.status() { BenchmarkRequestStatus::WaitingForArtifacts => unreachable!(), BenchmarkRequestStatus::ArtifactsReady => status_new::BenchmarkRequestStatus::Queued, - BenchmarkRequestStatus::InProgress => status_new::BenchmarkRequestStatus::InProgress, + BenchmarkRequestStatus::InProgress { .. } => { + status_new::BenchmarkRequestStatus::InProgress + } BenchmarkRequestStatus::Completed { .. } => { status_new::BenchmarkRequestStatus::Completed } @@ -150,6 +155,7 @@ fn request_to_ui( completed_at, duration_s, errors, + estimated_completed_at, } }