Store commit_date in benchmark_request

We need it to fill the `artifact` table.

Author: Kobzol

database/schema.md

@@ -284,6 +284,8 @@ Columns:
 * **parent_sha** (`text`): Parent SHA of the benchmarked commit.
   * Can be `NULL` for try requests without compiler artifacts.
 * **commit_type** (`text NOT NULL`): One of `master`, `try` or `release`.
+* **commit_date** (`timestamptz`): Datetime when the compiler artifact commit (not the request) was created.
+  * Can be `NULL` for try requests without compiler artifacts.
 * **pr** (`int`): Pull request number associated with the master/try commit.
   * `NULL` for release requests.
 * **created_at** (`timestamptz NOT NULL`): Datetime when the request was created.

database/src/lib.rs

@@ -889,6 +889,8 @@ impl fmt::Display for BenchmarkRequestType {
 #[derive(Debug, Clone, PartialEq)]
 pub struct BenchmarkRequest {
     commit_type: BenchmarkRequestType,
+    // When was the compiler artifact created
+    commit_date: Option<DateTime<Utc>>,
     created_at: DateTime<Utc>,
     status: BenchmarkRequestStatus,
     backends: String,
@@ -897,47 +899,45 @@ pub struct BenchmarkRequest {
 
 impl BenchmarkRequest {
     /// Create a release benchmark request that is in the `ArtifactsReady` status.
-    pub fn create_release(tag: &str, created_at: DateTime<Utc>) -> Self {
+    pub fn create_release(tag: &str, commit_date: DateTime<Utc>) -> Self {
         Self {
             commit_type: BenchmarkRequestType::Release {
                 tag: tag.to_string(),
             },
-            created_at,
+            commit_date: Some(commit_date),
+            created_at: Utc::now(),
             status: BenchmarkRequestStatus::ArtifactsReady,
             backends: String::new(),
             profiles: String::new(),
         }
     }
 
     /// Create a try request that is in the `WaitingForArtifacts` status.
-    pub fn create_try_without_artifacts(
-        pr: u32,
-        created_at: DateTime<Utc>,
-        backends: &str,
-        profiles: &str,
-    ) -> Self {
+    pub fn create_try_without_artifacts(pr: u32, backends: &str, profiles: &str) -> Self {
         Self {
             commit_type: BenchmarkRequestType::Try {
                 pr,
                 sha: None,
                 parent_sha: None,
             },
-            created_at,
+            commit_date: None,
+            created_at: Utc::now(),
             status: BenchmarkRequestStatus::WaitingForArtifacts,
             backends: backends.to_string(),
             profiles: profiles.to_string(),
         }
     }
 
     /// Create a master benchmark request that is in the `ArtifactsReady` status.
-    pub fn create_master(sha: &str, parent_sha: &str, pr: u32, created_at: DateTime<Utc>) -> Self {
+    pub fn create_master(sha: &str, parent_sha: &str, pr: u32, commit_date: DateTime<Utc>) -> Self {
         Self {
             commit_type: BenchmarkRequestType::Master {
                 pr,
                 sha: sha.to_string(),
                 parent_sha: parent_sha.to_string(),
             },
-            created_at,
+            commit_date: Some(commit_date),
+            created_at: Utc::now(),
             status: BenchmarkRequestStatus::ArtifactsReady,
             backends: String::new(),
             profiles: String::new(),

database/src/pool.rs

@@ -211,6 +211,7 @@ pub trait Connection: Send + Sync {
         pr: u32,
         sha: &str,
         parent_sha: &str,
+        commit_date: DateTime<Utc>,
     ) -> anyhow::Result<()>;
 
     /// Add a benchmark job to the job queue.
@@ -543,12 +544,12 @@ mod tests {
             // Complete
             let req_a = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", "");
             // WaitingForArtifacts
-            let req_b = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", "");
-            let req_c = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", "");
+            let req_b = BenchmarkRequest::create_try_without_artifacts(42, "", "");
+            let req_c = BenchmarkRequest::create_try_without_artifacts(42, "", "");
 
             db.insert_benchmark_request(&parent).await.unwrap();
             db.insert_benchmark_request(&req_a).await.unwrap();
-            db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1")
+            db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now())
                 .await
                 .unwrap();
 
@@ -615,7 +616,7 @@ mod tests {
             // ArtifactsReady
             let req_b = BenchmarkRequest::create_release("1.80.0", time);
             // WaitingForArtifacts
-            let req_c = BenchmarkRequest::create_try_without_artifacts(50, time, "", "");
+            let req_c = BenchmarkRequest::create_try_without_artifacts(50, "", "");
             // InProgress
             let req_d = BenchmarkRequest::create_master("sha-2", "parent-sha-2", 51, time);
             // Completed
@@ -649,10 +650,10 @@ mod tests {
             let db = ctx.db_client();
             let db = db.connection().await;
 
-            let req = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", "");
+            let req = BenchmarkRequest::create_try_without_artifacts(42, "", "");
 
             db.insert_benchmark_request(&req).await.unwrap();
-            db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1")
+            db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now())
                 .await
                 .unwrap();
 

database/src/pool/postgres.rs

@@ -378,6 +378,9 @@ static MIGRATIONS: &[&str] = &[
     ALTER TABLE job_queue DROP COLUMN IF EXISTS collector_id;
     CREATE INDEX IF NOT EXISTS job_queue_status_target_benchmark_set_idx ON job_queue (status, target, benchmark_set);
     "#,
+    r#"
+    ALTER TABLE benchmark_request ADD COLUMN commit_date TIMESTAMPTZ NULL;
+    "#,
 ];
 
 #[async_trait::async_trait]
@@ -1485,9 +1488,10 @@ where
                     status,
                     created_at,
                     backends,
-                    profiles
+                    profiles,
+                    commit_date
                 )
-                VALUES ($1, $2, $3, $4, $5, $6, $7, $8);
+                VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9);
             "#,
                 &[
                     &benchmark_request.tag(),
@@ -1498,6 +1502,7 @@ where
                     &benchmark_request.created_at,
                     &benchmark_request.backends,
                     &benchmark_request.profiles,
+                    &benchmark_request.commit_date,
                 ],
             )
             .await
@@ -1563,6 +1568,7 @@ where
         pr: u32,
         sha: &str,
         parent_sha: &str,
+        commit_date: DateTime<Utc>,
     ) -> anyhow::Result<()> {
         self.conn()
             .execute(
@@ -1571,7 +1577,8 @@ where
             SET
                 tag = $1,
                 parent_sha = $2,
-                status = $3
+                status = $3,
+                commit_date = $6
             WHERE
                 pr = $4
                 AND commit_type = 'try'
@@ -1583,6 +1590,7 @@ where
                     &BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR,
                     &(pr as i32),
                     &BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR,
+                    &commit_date,
                 ],
             )
             .await
@@ -1603,7 +1611,8 @@ where
                     created_at,
                     completed_at,
                     backends,
-                    profiles
+                    profiles,
+                    commit_date
                 FROM benchmark_request
                 WHERE status IN('{BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR}', '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}')"#
         );
@@ -1626,6 +1635,7 @@ where
                 let completed_at = row.get::<_, Option<DateTime<Utc>>>(6);
                 let backends = row.get::<_, String>(7);
                 let profiles = row.get::<_, String>(8);
+                let commit_date = row.get::<_, Option<DateTime<Utc>>>(9);
 
                 let pr = pr.map(|v| v as u32);
 
@@ -1640,6 +1650,7 @@ where
                             parent_sha,
                             pr: pr.expect("Try commit in the DB without a PR"),
                         },
+                        commit_date,
                         created_at,
                         status,
                         backends,
@@ -1652,6 +1663,7 @@ where
                                 .expect("Master commit in the DB without a parent SHA"),
                             pr: pr.expect("Master commit in the DB without a PR"),
                         },
+                        commit_date,
                         created_at,
                         status,
                         backends,
@@ -1661,6 +1673,7 @@ where
                         commit_type: BenchmarkRequestType::Release {
                             tag: tag.expect("Release commit in the DB without a SHA"),
                         },
+                        commit_date,
                         created_at,
                         status,
                         backends,

database/src/pool/sqlite.rs

@@ -1289,6 +1289,7 @@ impl Connection for SqliteConnection {
         _pr: u32,
         _sha: &str,
         _parent_sha: &str,
+        _commit_date: DateTime<Utc>,
     ) -> anyhow::Result<()> {
         no_queue_implementation_abort!()
     }

site/src/github.rs

@@ -4,11 +4,11 @@ pub mod comparison_summary;
 use crate::api::github::Commit;
 use crate::job_queue::run_new_queue;
 use crate::load::{MissingReason, SiteCtxt, TryCommit};
+use chrono::{DateTime, Utc};
+use serde::Deserialize;
 use std::sync::LazyLock;
 use std::time::Duration;
 
-use serde::Deserialize;
-
 type BoxedError = Box<dyn std::error::Error + Send + Sync>;
 
 pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust";
@@ -236,13 +236,18 @@ pub async fn rollup_pr_number(
 
 async fn attach_shas_to_try_benchmark_request(
     conn: &dyn database::pool::Connection,
-    pr: u32,
-    sha: &str,
-    parent_sha: &str,
+    pr_number: u32,
+    commit: &TryCommit,
+    commit_date: DateTime<Utc>,
 ) {
     if run_new_queue() {
         if let Err(e) = conn
-            .attach_shas_to_try_benchmark_request(pr, sha, parent_sha)
+            .attach_shas_to_try_benchmark_request(
+                pr_number,
+                &commit.sha,
+                &commit.parent_sha,
+                commit_date,
+            )
             .await
         {
             log::error!("Failed to add shas to try commit {}", e);
@@ -279,8 +284,8 @@ pub async fn enqueue_shas(
         attach_shas_to_try_benchmark_request(
             &*conn,
             pr_number,
-            &try_commit.sha,
-            &try_commit.parent_sha,
+            &try_commit,
+            commit_response.commit.committer.date,
         )
         .await;
 

site/src/job_queue/mod.rs

@@ -68,9 +68,9 @@ async fn create_benchmark_request_releases(
         .take(20)
         .collect();
 
-    for (name, date_time) in releases {
-        if date_time >= cutoff && !index.contains_tag(&name) {
-            let release_request = BenchmarkRequest::create_release(&name, date_time);
+    for (name, commit_date) in releases {
+        if commit_date >= cutoff && !index.contains_tag(&name) {
+            let release_request = BenchmarkRequest::create_release(&name, commit_date);
             log::info!("Inserting release benchmark request {release_request:?}");
             if let Err(error) = conn.insert_benchmark_request(&release_request).await {
                 log::error!("Failed to insert release benchmark request: {error}");
@@ -305,43 +305,21 @@ pub async fn cron_main(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, seconds: u
 
 #[cfg(test)]
 mod tests {
-    use super::*;
-    use chrono::{Datelike, Duration, TimeZone, Utc};
-    use database::{
-        tests::run_postgres_test, BenchmarkJobConclusion, BenchmarkSet, CodegenBackend, Profile,
-    };
-
-    fn days_ago(day_str: &str) -> chrono::DateTime<Utc> {
-        // Walk backwards until the first non-digit, then slice
-        let days = day_str
-            .strip_prefix("days")
-            .unwrap()
-            .parse::<i64>()
-            .unwrap();
-
-        let timestamp = Utc::now() - Duration::days(days);
-        // zero out the seconds
-        Utc.with_ymd_and_hms(
-            timestamp.year(),
-            timestamp.month(),
-            timestamp.day(),
-            0,
-            0,
-            0,
-        )
-        .unwrap()
-    }
+    use crate::job_queue::build_queue;
+    use chrono::Utc;
+    use database::tests::run_postgres_test;
+    use database::{BenchmarkRequest, BenchmarkRequestStatus};
 
-    fn create_master(sha: &str, parent: &str, pr: u32, age_days: &str) -> BenchmarkRequest {
-        BenchmarkRequest::create_master(sha, parent, pr, days_ago(age_days))
+    fn create_master(sha: &str, parent: &str, pr: u32) -> BenchmarkRequest {
+        BenchmarkRequest::create_master(sha, parent, pr, Utc::now())
     }
 
-    fn create_try(pr: u32, age_days: &str) -> BenchmarkRequest {
-        BenchmarkRequest::create_try_without_artifacts(pr, days_ago(age_days), "", "")
+    fn create_try(pr: u32) -> BenchmarkRequest {
+        BenchmarkRequest::create_try_without_artifacts(pr, "", "")
     }
 
-    fn create_release(tag: &str, age_days: &str) -> BenchmarkRequest {
-        BenchmarkRequest::create_release(tag, days_ago(age_days))
+    fn create_release(tag: &str) -> BenchmarkRequest {
+        BenchmarkRequest::create_release(tag, Utc::now())
     }
 
     async fn db_insert_requests(
@@ -482,26 +460,26 @@ mod tests {
              *           +----------------+
              **/
             let requests = vec![
-                create_master("bar", "parent1", 10, "days2"),
-                create_master("345", "parent2", 11, "days2"),
-                create_master("aaa", "parent3", 12, "days2"),
-                create_master("rrr", "parent4", 13, "days2"),
-                create_master("123", "bar", 14, "days2"),
-                create_master("foo", "345", 15, "days2"),
-                create_try(16, "days1"),
-                create_release("v1.2.3", "days2"),
-                create_try(17, "days1"),
-                create_master("mmm", "aaa", 18, "days2"),
+                create_master("bar", "parent1", 10),
+                create_master("345", "parent2", 11),
+                create_master("aaa", "parent3", 12),
+                create_master("rrr", "parent4", 13),
+                create_master("123", "bar", 14),
+                create_master("foo", "345", 15),
+                create_try(16),
+                create_release("v1.2.3"),
+                create_try(17),
+                create_master("mmm", "aaa", 18),
             ];
 
             db_insert_requests(&*db, &requests).await;
-            db.attach_shas_to_try_benchmark_request(16, "try1", "rrr")
+            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.attach_shas_to_try_benchmark_request(17, "baz", "foo")
+            db.attach_shas_to_try_benchmark_request(17, "baz", "foo", Utc::now())
                 .await
                 .unwrap();
 

site/src/request_handlers/github.rs

@@ -84,8 +84,7 @@ async fn record_try_benchmark_request_without_artifacts(
 ) {
     // We only want to run this if the new system is running
     if run_new_queue() {
-        let try_request =
-            BenchmarkRequest::create_try_without_artifacts(pr, chrono::Utc::now(), backends, "");
+        let try_request = BenchmarkRequest::create_try_without_artifacts(pr, backends, "");
         log::info!("Inserting try benchmark request {try_request:?}");
         if let Err(e) = conn.insert_benchmark_request(&try_request).await {
             log::error!("Failed to insert try benchmark request: {}", e);