Add function for getting the last N completed requests

Kobzol · Kobzol · commit f75d4a2e2132 · 2025-08-29T16:12:13.000+02:00
The function returns errors associated with the requests, including both the error benchmark/context name and the error content. The query is also prepared, because it should be as fast as possible to not block the status page.

Also adds some basic test builders for easier preparation of test state in tests.
diff --git a/database/src/lib.rs b/database/src/lib.rs
@@ -1248,6 +1248,13 @@ pub struct InProgressRequestWithJobs {
     pub parent: Option<(BenchmarkRequest, Vec<BenchmarkJob>)>,
 }
 
+#[derive(Debug, PartialEq)]
+pub struct CompletedBenchmarkRequestWithErrors {
+    request: BenchmarkRequest,
+    /// Benchmark (name) -> error
+    errors: HashMap<String, String>,
+}
+
 /// The data that can be retrived from the database directly to populate the
 /// status page
 #[derive(Debug, PartialEq)]
diff --git a/database/src/pool.rs b/database/src/pool.rs
@@ -2,7 +2,8 @@ use crate::selector::CompileTestCase;
 use crate::{
     ArtifactCollection, ArtifactId, ArtifactIdNumber, BenchmarkJob, BenchmarkJobConclusion,
     BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkSet, CodegenBackend,
-    CollectorConfig, CompileBenchmark, PartialStatusPageData, Target,
+    CollectorConfig, CompileBenchmark, CompletedBenchmarkRequestWithErrors, PartialStatusPageData,
+    Target,
 };
 use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step};
 use chrono::{DateTime, Utc};
@@ -277,6 +278,15 @@ pub trait Connection: Send + Sync {
 
     async fn get_status_page_data(&self) -> anyhow::Result<PartialStatusPageData>;
 
+    /// Return the last `count` completed benchmark requests, along with all errors associated with
+    /// them.
+    ///
+    /// The requests will be ordered from most recently to least recently completed.
+    async fn get_last_n_completed_benchmark_requests(
+        &self,
+        count: u64,
+    ) -> anyhow::Result<Vec<CompletedBenchmarkRequestWithErrors>>;
+
     /// Get all of the configuration for all of the collectors
     async fn get_collector_configs(&self) -> anyhow::Result<Vec<CollectorConfig>>;
 
@@ -403,6 +413,7 @@ impl Pool {
 mod tests {
     use super::*;
     use crate::metric::Metric;
+    use crate::tests::builder::{JobBuilder, RequestBuilder};
     use crate::tests::run_postgres_test;
     use crate::BenchmarkJobStatus;
     use crate::{tests::run_db_test, BenchmarkRequestType, Commit, CommitType, Date};
@@ -1190,4 +1201,80 @@ mod tests {
         })
         .await;
     }
+
+    #[tokio::test]
+    async fn get_last_completed_requests() {
+        run_postgres_test(|ctx| async {
+            let mut requests = vec![];
+            let db = ctx.db();
+
+            let collector = ctx.add_collector(Default::default()).await;
+
+            // Create several completed requests
+            for id in 1..=3 {
+                // Make some space between completions
+                tokio::time::sleep(Duration::from_millis(100)).await;
+
+                requests.push(
+                    RequestBuilder::master(
+                        db,
+                        &format!("sha{}", id),
+                        &format!("sha{}", id - 1),
+                        id,
+                    )
+                    .await
+                    .add_job(db, JobBuilder::new())
+                    .await
+                    .complete(db, &collector)
+                    .await,
+                );
+            }
+
+            // Create an additional non-completed request
+            ctx.insert_master_request("foo", "bar", 1000).await;
+
+            // Request 1 will have artifact with errors
+            let aid1 = ctx.upsert_master_artifact("sha1").await;
+            db.record_error(aid1, "crate1", "error1").await;
+            db.record_error(aid1, "crate2", "error2").await;
+
+            // Request 2 will have artifact without errors
+            let _aid2 = ctx.upsert_master_artifact("sha2").await;
+
+            // Request 3 will have no artifact (shouldn't happen in practice, but...)
+
+            let reqs = db.get_last_n_completed_benchmark_requests(5).await.unwrap();
+            assert_eq!(reqs.len(), 3);
+
+            let expected = [
+                ("sha3", HashMap::new()),
+                ("sha2", HashMap::new()),
+                (
+                    "sha1",
+                    HashMap::from([
+                        ("crate1".to_string(), "error1".to_string()),
+                        ("crate2".to_string(), "error2".to_string()),
+                    ]),
+                ),
+            ];
+            for ((sha, errors), req) in expected.into_iter().zip(reqs) {
+                assert_eq!(
+                    req.request.tag().unwrap(),
+                    sha,
+                    "Request {req:?} does not have expected sha {sha}"
+                );
+                assert_eq!(
+                    req.errors, errors,
+                    "Request {req:?} does not have expected errors {errors:?}"
+                );
+            }
+
+            let reqs = db.get_last_n_completed_benchmark_requests(1).await.unwrap();
+            assert_eq!(reqs.len(), 1);
+            assert_eq!(reqs[0].request.tag().unwrap(), "sha3");
+
+            Ok(ctx)
+        })
+        .await;
+    }
 }
diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs
@@ -4,9 +4,9 @@ use crate::{
     ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkJob,
     BenchmarkJobConclusion, BenchmarkJobStatus, BenchmarkRequest, BenchmarkRequestIndex,
     BenchmarkRequestStatus, BenchmarkRequestType, BenchmarkSet, CodegenBackend, CollectionId,
-    CollectorConfig, Commit, CommitType, CompileBenchmark, Date, InProgressRequestWithJobs, Index,
-    PartialStatusPageData, Profile, QueuedCommit, Scenario, Target,
-    BENCHMARK_JOB_STATUS_FAILURE_STR, BENCHMARK_JOB_STATUS_IN_PROGRESS_STR,
+    CollectorConfig, Commit, CommitType, CompileBenchmark, CompletedBenchmarkRequestWithErrors,
+    Date, InProgressRequestWithJobs, Index, PartialStatusPageData, Profile, QueuedCommit, Scenario,
+    Target, BENCHMARK_JOB_STATUS_FAILURE_STR, BENCHMARK_JOB_STATUS_IN_PROGRESS_STR,
     BENCHMARK_JOB_STATUS_QUEUED_STR, BENCHMARK_JOB_STATUS_SUCCESS_STR,
     BENCHMARK_REQUEST_MASTER_STR, BENCHMARK_REQUEST_RELEASE_STR,
     BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR, BENCHMARK_REQUEST_STATUS_COMPLETED_STR,
@@ -471,6 +471,7 @@ pub struct CachedStatements {
     get_artifact_size: Statement,
     load_benchmark_request_index: Statement,
     get_compile_test_cases_with_measurements: Statement,
+    get_last_n_completed_requests_with_errors: Statement,
 }
 
 pub struct PostgresTransaction<'a> {
@@ -667,12 +668,45 @@ impl PostgresConnection {
                         WHERE aid = $1
                     )
                 ").await.unwrap(),
+                get_last_n_completed_requests_with_errors: conn.prepare(&format!("
+                    WITH completed AS (
+                        SELECT {BENCHMARK_REQUEST_COLUMNS}
+                        FROM benchmark_request
+                        WHERE status = $1
+                        -- Select last N completed requests
+                        ORDER BY completed_at DESC
+                        LIMIT $2
+                    ), artifacts AS (
+                        SELECT artifact.id, name
+                        FROM artifact
+                        -- Use right join to only return artifacts for selected requests
+                        RIGHT JOIN completed ON artifact.name = completed.tag
+                    ), errors AS (
+                        SELECT
+                            artifacts.name AS tag,
+                            error.benchmark,
+                            error.error
+                        FROM error
+                        -- Use right join to only return errors for selected artifacts
+                        RIGHT JOIN artifacts ON error.aid = artifacts.id
+                    )
+                    -- Select request duplicated for each pair of (benchmark, error)
+                    SELECT
+                        completed.*,
+                        errors.benchmark,
+                        errors.error
+                    FROM completed
+                    LEFT JOIN errors ON errors.tag = completed.tag
+                    -- Resort the requests, because the original order may be lost
+                    ORDER BY completed.completed_at DESC;
+                ")).await.unwrap(),
             }),
             conn,
         }
     }
 }
 
+// `tag` should be kept as the first column
 const BENCHMARK_REQUEST_COLUMNS: &str =
     "tag, parent_sha, pr, commit_type, status, created_at, completed_at, backends, profiles, commit_date, duration_ms";
 
@@ -1986,6 +2020,56 @@ where
         Ok(())
     }
 
+    async fn get_last_n_completed_benchmark_requests(
+        &self,
+        count: u64,
+    ) -> anyhow::Result<Vec<CompletedBenchmarkRequestWithErrors>> {
+        let rows = self
+            .conn()
+            .query(
+                &self.statements().get_last_n_completed_requests_with_errors,
+                &[&BENCHMARK_REQUEST_STATUS_COMPLETED_STR, &(count as i64)],
+            )
+            .await?;
+
+        // Iterate through the requests and aggregate their errors
+        // Make sure to keep their original order
+        let mut requests = vec![];
+        // tag -> errors
+        let mut errors: HashMap<String, HashMap<String, String>> = Default::default();
+
+        for row in rows {
+            let tag = row.get::<_, &str>(0);
+            let error_benchmark = row.get::<_, Option<String>>(11);
+            let error_content = row.get::<_, Option<String>>(12);
+
+            // We already saw this request, just add errors
+            if let Some(errors) = errors.get_mut(tag) {
+                if let Some(benchmark) = error_benchmark {
+                    errors.insert(benchmark, error_content.unwrap_or_default());
+                }
+            } else {
+                // We see this request for the first time
+                let request = row_to_benchmark_request(&row, None);
+                let request_errors = if let Some(benchmark) = error_benchmark {
+                    HashMap::from([(benchmark, error_content.unwrap_or_default())])
+                } else {
+                    HashMap::new()
+                };
+                errors.insert(tag.to_string(), request_errors);
+                requests.push(request);
+            }
+        }
+
+        Ok(requests
+            .into_iter()
+            .map(|request| {
+                let errors = errors.remove(request.tag().unwrap()).unwrap_or_default();
+                CompletedBenchmarkRequestWithErrors { request, errors }
+            })
+            .collect())
+    }
+
     async fn get_status_page_data(&self) -> anyhow::Result<PartialStatusPageData> {
         let max_completed_requests = 30;
 
@@ -2273,7 +2357,9 @@ fn row_to_benchmark_request(row: &Row, row_offset: Option<usize>) -> BenchmarkRe
 
     let status =
         BenchmarkRequestStatus::from_str_and_completion_date(status, completed_at, duration_ms)
-            .expect("Invalid BenchmarkRequestStatus data in the database");
+            .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
@@ -3,8 +3,8 @@ use crate::selector::CompileTestCase;
 use crate::{
     ArtifactCollection, ArtifactId, Benchmark, BenchmarkJob, BenchmarkJobConclusion,
     BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkSet, CodegenBackend,
-    CollectionId, CollectorConfig, Commit, CommitType, CompileBenchmark, Date,
-    PartialStatusPageData, Profile, Target,
+    CollectionId, CollectorConfig, Commit, CommitType, CompileBenchmark,
+    CompletedBenchmarkRequestWithErrors, Date, PartialStatusPageData, Profile, Target,
 };
 use crate::{ArtifactIdNumber, Index, QueuedCommit};
 use chrono::{DateTime, TimeZone, Utc};
@@ -1382,6 +1382,13 @@ impl Connection for SqliteConnection {
     async fn update_collector_heartbeat(&self, _collector_name: &str) -> anyhow::Result<()> {
         no_queue_implementation_abort!()
     }
+
+    async fn get_last_n_completed_benchmark_requests(
+        &self,
+        _count: u64,
+    ) -> anyhow::Result<Vec<CompletedBenchmarkRequestWithErrors>> {
+        no_queue_implementation_abort!()
+    }
 }
 
 fn parse_artifact_id(ty: &str, sha: &str, date: Option<i64>) -> ArtifactId {
diff --git a/database/src/tests/builder.rs b/database/src/tests/builder.rs
diff --git a/database/src/tests/mod.rs b/database/src/tests/mod.rs
