PR feedback & tests

Author: Jamesbarford

database/src/lib.rs

@@ -799,28 +799,10 @@ pub struct ArtifactCollection {
     pub end_time: DateTime<Utc>,
 }
 
-#[derive(Debug)]
-pub enum BenchmarkRequestCommitType {
-    Try { pr: u32 },
-    Master { pr: u32 },
-    Release { tag: String },
-}
-
-impl fmt::Display for BenchmarkRequestCommitType {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            BenchmarkRequestCommitType::Try { pr: _ } => write!(f, "try"),
-            BenchmarkRequestCommitType::Master { pr: _ } => write!(f, "master"),
-            BenchmarkRequestCommitType::Release { tag: _ } => write!(f, "release"),
-        }
-    }
-}
-
 #[derive(Debug)]
 pub enum BenchmarkRequestStatus {
     WaitingForArtifacts,
     WaitingForParent,
-    Queued,
     InProgress,
     Completed,
 }
@@ -830,7 +812,6 @@ impl fmt::Display for BenchmarkRequestStatus {
         match self {
             BenchmarkRequestStatus::WaitingForArtifacts => write!(f, "waiting_for_artifacts"),
             BenchmarkRequestStatus::WaitingForParent => write!(f, "waiting_for_parent"),
-            BenchmarkRequestStatus::Queued => write!(f, "queued"),
             BenchmarkRequestStatus::InProgress => write!(f, "in_progress"),
             BenchmarkRequestStatus::Completed => write!(f, "completed"),
         }
@@ -855,6 +836,24 @@ pub enum BenchmarkRequestType {
     Release { tag: String },
 }
 
+impl BenchmarkRequestType {
+    pub fn commit_type_str(&self) -> &str {
+        match self {
+            BenchmarkRequestType::Try {
+                sha: _,
+                parent_sha: _,
+                pr: _,
+            } => "try",
+            BenchmarkRequestType::Master {
+                sha: _,
+                parent_sha: _,
+                pr: _,
+            } => "master",
+            BenchmarkRequestType::Release { tag: _ } => "release",
+        }
+    }
+}
+
 impl fmt::Display for BenchmarkRequestType {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -948,4 +947,61 @@ impl BenchmarkRequest {
             profiles: profiles.to_string(),
         }
     }
+
+    /// Get either the `sha` for a `try` or `master` commit or a `tag` for a
+    /// `release`
+    pub fn tag(&self) -> &str {
+        match &self.commit_type {
+            BenchmarkRequestType::Try {
+                sha,
+                parent_sha: _,
+                pr: _,
+            }
+            | BenchmarkRequestType::Master {
+                sha,
+                parent_sha: _,
+                pr: _,
+            } => sha,
+            BenchmarkRequestType::Release { tag } => tag,
+        }
+    }
+
+    pub fn pr(&self) -> Option<&u32> {
+        match &self.commit_type {
+            BenchmarkRequestType::Try {
+                sha: _,
+                parent_sha: _,
+                pr,
+            }
+            | BenchmarkRequestType::Master {
+                sha: _,
+                parent_sha: _,
+                pr,
+            } => Some(pr),
+            BenchmarkRequestType::Release { tag: _ } => None,
+        }
+    }
+
+    pub fn commit_type(&self) -> &str {
+        self.commit_type.commit_type_str()
+    }
+
+    pub fn parent_sha(&self) -> Option<&str> {
+        match &self.commit_type {
+            BenchmarkRequestType::Try {
+                sha: _,
+                parent_sha,
+                pr: _,
+            }
+            | BenchmarkRequestType::Master {
+                sha: _,
+                parent_sha,
+                pr: _,
+            } => match parent_sha {
+                Some(parent_sha) => Some(parent_sha),
+                None => None,
+            },
+            BenchmarkRequestType::Release { tag: _ } => None,
+        }
+    }
 }

database/src/pool.rs

@@ -306,7 +306,7 @@ mod tests {
     use std::str::FromStr;
 
     use super::*;
-    use crate::{tests::run_db_test, Commit, CommitType, Date};
+    use crate::{tests::run_db_test, BenchmarkRequestStatus, Commit, CommitType, Date};
 
     /// Create a Commit
     fn create_commit(commit_sha: &str, time: chrono::DateTime<Utc>, r#type: CommitType) -> Commit {
@@ -375,4 +375,50 @@ mod tests {
         })
         .await;
     }
+
+    #[tokio::test]
+    async fn insert_benchmark_requests() {
+        run_db_test(|ctx| async {
+            let db = ctx.db_client();
+            let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap();
+            let master_benchmark_request = BenchmarkRequest::create_master(
+                "a-sha-1",
+                Some("parent-sha-1"),
+                42,
+                time,
+                BenchmarkRequestStatus::WaitingForParent,
+                "llvm",
+                "",
+            );
+
+            let try_benchmark_request = BenchmarkRequest::create_try(
+                "b-sha-2",
+                Some("parent-sha-2"),
+                32,
+                time,
+                BenchmarkRequestStatus::WaitingForParent,
+                "cranelift",
+                "",
+            );
+
+            let release_benchmark_request = BenchmarkRequest::create_release(
+                "1.8.0",
+                time,
+                BenchmarkRequestStatus::WaitingForParent,
+                "cranelift,llvm",
+                "",
+            );
+
+            let db = db.connection().await;
+            db.insert_benchmark_request(&master_benchmark_request).await;
+            db.insert_benchmark_request(&try_benchmark_request).await;
+            db.insert_benchmark_request(&release_benchmark_request)
+                .await;
+            // duplicate insert
+            db.insert_benchmark_request(&master_benchmark_request).await;
+
+            Ok(ctx)
+        })
+        .await;
+    }
 }

database/src/pool/postgres.rs

@@ -1,8 +1,8 @@
 use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction};
 use crate::{
-    ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkRequest,
-    BenchmarkRequestType, CodegenBackend, CollectionId, Commit, CommitType, CompileBenchmark, Date,
-    Index, Profile, QueuedCommit, Scenario, Target,
+    ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkRequest, CodegenBackend,
+    CollectionId, Commit, CommitType, CompileBenchmark, Date, Index, Profile, QueuedCommit,
+    Scenario, Target,
 };
 use anyhow::Context as _;
 use chrono::{DateTime, TimeZone, Utc};
@@ -287,20 +287,20 @@ static MIGRATIONS: &[&str] = &[
     alter table pstat_series add constraint test_case UNIQUE(crate, profile, scenario, backend, target, metric);
     "#,
     r#"
-    CREATE TABLE IF NOT EXISTS benchmark_requests (
+    CREATE TABLE IF NOT EXISTS benchmark_request (
         id           SERIAL PRIMARY KEY,
         tag          TEXT NOT NULL UNIQUE,
         parent_sha   TEXT,
-        commit_type  TEXT,
+        commit_type  TEXT NOT NULL,
         pr           INTEGER,
         created_at   TIMESTAMPTZ NOT NULL,
         completed_at TIMESTAMPTZ,
         status       TEXT NOT NULL,
-        backends     TEXT,
-        profiles     TEXT
+        backends     TEXT NOT NULL,
+        profiles     TEXT NOT NULL
     );
-    CREATE INDEX IF NOT EXISTS benchmark_requests_status_idx on benchmark_requests (status);
-    CREATE INDEX IF NOT EXISTS benchmark_requests_commit_type on benchmark_requests (commit_type);
+    CREATE INDEX IF NOT EXISTS benchmark_request_status_idx on benchmark_request (status);
+    CREATE INDEX IF NOT EXISTS benchmark_request_commit_type on benchmark_request (commit_type);
     "#,
 ];
 
@@ -1384,63 +1384,35 @@ where
     }
 
     async fn insert_benchmark_request(&self, benchmark_request: &BenchmarkRequest) {
-        match &benchmark_request.commit_type {
-            BenchmarkRequestType::Try {
-                sha,
-                parent_sha,
-                pr,
-            }
-            | BenchmarkRequestType::Master {
-                sha,
-                parent_sha,
-                pr,
-            } => {
-                self.conn()
-                    .execute(
-                        r#"
-                        INSERT INTO
-                           benchmark_requests(parent_sha, pr, tag, created_at,
-                                              commit_type, status, backends, profiles)
-                        VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
-                        ON CONFLICT DO NOTHING;
-                        "#,
-                        &[
-                            &parent_sha,
-                            pr,
-                            sha,
-                            &benchmark_request.created_at,
-                            &benchmark_request.commit_type.to_string(),
-                            &benchmark_request.status.to_string(),
-                            &benchmark_request.backends,
-                            &benchmark_request.profiles,
-                        ],
-                    )
-                    .await
-                    .unwrap();
-            }
-            BenchmarkRequestType::Release { tag } => {
-                self.conn()
-                    .execute(
-                        r#"
-                        INSERT INTO
-                            benchmark_requests(tag, created_at, commit_type,
-                                               status, backends, profiles)
-                        VALUES ($1, $2, $3, $4, $5, $6);
-                        ON CONFLICT DO NOTHING;
-                        "#,
-                        &[
-                            tag,
-                            &benchmark_request.created_at,
-                            &benchmark_request.commit_type.to_string(),
-                            &benchmark_request.status.to_string(),
-                            &benchmark_request.backends,
-                            &benchmark_request.profiles,
-                        ],
-                    )
-                    .await
-                    .unwrap();
-            }
-        }
+        self.conn()
+            .execute(
+                r#"
+                INSERT INTO benchmark_request(
+                    tag,
+                    parent_sha,
+                    pr,
+                    commit_type,
+                    status,
+                    created_at,
+                    backends,
+                    profiles
+                )
+                VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+                ON CONFLICT DO NOTHING;
+            "#,
+                &[
+                    &benchmark_request.tag(),
+                    &benchmark_request.parent_sha(),
+                    &benchmark_request.pr().map(|it| *it as i32),
+                    &benchmark_request.commit_type(),
+                    &benchmark_request.status.to_string(),
+                    &benchmark_request.created_at,
+                    &benchmark_request.backends,
+                    &benchmark_request.profiles,
+                ],
+            )
+            .await
+            .unwrap();
     }
 }
 

database/src/pool/sqlite.rs

@@ -1254,7 +1254,7 @@ impl Connection for SqliteConnection {
     }
 
     async fn insert_benchmark_request(&self, _benchmark_request: &BenchmarkRequest) {
-        panic!("Queueing for SQLite has not been implemented, if you are wanting to test the queueing functionality please use postgres. Presuming you have docker installed, at the root of the repo you can run `make start-postgres` to spin up a postrgres database.");
+        panic!("Queueing for SQLite has not been implemented, if you are wanting to test the queueing functionality please use postgres. Presuming you have docker installed, at the root of the repo you can run `make start-postgres` to spin up a postgres database.");
     }
 }
 

database/src/tests/mod.rs

@@ -126,6 +126,9 @@ where
         let ctx = TestContext::new_postgres(&db_url).await;
         let ctx = f(ctx).await.expect("Postgres test failed");
         ctx.finish().await;
+        // We don't want to run both postgres tests & sqlite tests as some tests
+        // are only designed for postgres
+        return;
     } else {
         // The github CI does not yet support running containers on Windows,
         // meaning that the test suite would fail.

site/src/job_queue.rs

@@ -0,0 +1,59 @@
+use std::{str::FromStr, sync::Arc};
+
+use crate::load::SiteCtxt;
+use chrono::Utc;
+use database::{BenchmarkRequest, BenchmarkRequestStatus};
+use parking_lot::RwLock;
+use tokio::time::{self, Duration};
+
+impl SiteCtxt {
+    /// Store the latest master commits or do nothing if all of them are
+    /// already in the database
+    pub async fn enqueue_master_commits(&self) {
+        let conn = self.conn().await;
+        let master_commits = &self.get_master_commits().commits;
+        // TODO; delete at some point in the future
+        let cutoff: chrono::DateTime<Utc> =
+            chrono::DateTime::from_str("2025-06-01T00:00:00.000Z").unwrap();
+
+        for master_commit in master_commits {
+            // We don't want to add masses of obsolete data
+            if master_commit.time >= cutoff {
+                let pr = master_commit.pr.unwrap_or(0);
+                let benchmark = BenchmarkRequest::create_master(
+                    &master_commit.sha,
+                    Some(&master_commit.parent_sha),
+                    pr,
+                    master_commit.time,
+                    BenchmarkRequestStatus::WaitingForParent,
+                    "",
+                    "",
+                );
+                conn.insert_benchmark_request(&benchmark).await;
+            }
+        }
+    }
+}
+
+/// Inserts and manages the queue at `seconds` interval
+async fn cron_enqueue_jobs(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, seconds: u64) {
+    let mut interval = time::interval(Duration::from_secs(seconds));
+
+    loop {
+        let ctxt = site_ctxt.clone();
+        if let Some(ctxt_clone) = {
+            let guard = ctxt.read();
+            guard.as_ref().cloned()
+        } {
+            ctxt_clone.enqueue_master_commits().await;
+        }
+
+        interval.tick().await;
+        println!("Cron job executed at: {:?}", std::time::SystemTime::now());
+    }
+}
+
+/// Entry point for the cron
+pub async fn cron_main(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, seconds: u64) {
+    cron_enqueue_jobs(site_ctxt, seconds).await;
+}