Disable creating master/release benchmark requests and enqueuing parent jobs

Kobzol · Kobzol · commit 21929934668d · 2025-09-25T18:42:03.000+02:00
diff --git a/site/src/github.rs b/site/src/github.rs
@@ -2,7 +2,7 @@ pub mod client;
 pub mod comparison_summary;
 
 use crate::api::github::Commit;
-use crate::job_queue::{run_new_queue, should_use_new_system};
+use crate::job_queue::should_use_new_system;
 use crate::load::{MissingReason, SiteCtxt, TryCommit};
 use chrono::{DateTime, Utc};
 use serde::Deserialize;
diff --git a/site/src/job_queue/mod.rs b/site/src/job_queue/mod.rs
@@ -32,7 +32,7 @@ pub fn should_use_new_system(pr: u32) -> bool {
 /// Returns `true` if at least one benchmark request was inserted.
 async fn create_benchmark_request_master_commits(
     ctxt: &SiteCtxt,
-    conn: &dyn database::pool::Connection,
+    _conn: &dyn database::pool::Connection,
     index: &BenchmarkRequestIndex,
 ) -> anyhow::Result<bool> {
     let now = Utc::now();
@@ -47,7 +47,7 @@ async fn create_benchmark_request_master_commits(
     // TODO; delete at some point in the future
     let cutoff: chrono::DateTime<Utc> = chrono::DateTime::from_str("2025-08-27T00:00:00.000Z")?;
 
-    let mut inserted = false;
+    let inserted = false;
     for master_commit in master_commits {
         // We don't want to add masses of obsolete data
         if master_commit.time >= cutoff && !index.contains_tag(&master_commit.sha) {
@@ -59,11 +59,14 @@ async fn create_benchmark_request_master_commits(
                 master_commit.time,
             );
             log::info!("Inserting master benchmark request {benchmark:?}");
-            if let Err(error) = conn.insert_benchmark_request(&benchmark).await {
-                log::error!("Failed to insert master benchmark request: {error:?}");
-            } else {
-                inserted = true;
-            }
+
+            // Do not create benchmark requests on production, to allow running in parallel with
+            // the old system.
+            // if let Err(error) = conn.insert_benchmark_request(&benchmark).await {
+            //     log::error!("Failed to insert master benchmark request: {error:?}");
+            // } else {
+            //     inserted = true;
+            // }
         }
     }
     Ok(inserted)
@@ -73,7 +76,7 @@ async fn create_benchmark_request_master_commits(
 /// already in the database
 /// Returns `true` if at least one benchmark request was inserted.
 async fn create_benchmark_request_releases(
-    conn: &dyn database::pool::Connection,
+    _conn: &dyn database::pool::Connection,
     index: &BenchmarkRequestIndex,
 ) -> anyhow::Result<bool> {
     let releases: String = reqwest::get("https://static.rust-lang.org/manifests.txt")
@@ -89,16 +92,19 @@ async fn create_benchmark_request_releases(
         .filter_map(parse_release_string)
         .take(20);
 
-    let mut inserted = false;
+    let inserted = false;
     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}");
-            } else {
-                inserted = true;
-            }
+
+            // Do not create benchmark requests on production, to allow running in parallel with
+            // the old system.
+            // if let Err(error) = conn.insert_benchmark_request(&release_request).await {
+            //     log::error!("Failed to insert release benchmark request: {error}");
+            // } else {
+            //     inserted = true;
+            // }
         }
     }
     Ok(inserted)
@@ -211,18 +217,18 @@ pub async fn build_queue(
 /// This is performed atomically, in a transaction.
 pub async fn enqueue_benchmark_request(
     conn: &mut dyn database::pool::Connection,
-    benchmark_request: &BenchmarkRequest,
+    request: &BenchmarkRequest,
 ) -> anyhow::Result<()> {
     let mut tx = conn.transaction().await;
 
-    let Some(request_tag) = benchmark_request.tag() else {
-        panic!("Benchmark request {benchmark_request:?} has no tag");
+    let Some(request_tag) = request.tag() else {
+        panic!("Benchmark request {request:?} has no tag");
     };
 
-    log::info!("Enqueuing jobs for request {benchmark_request:?}");
+    log::info!("Enqueuing jobs for request {request:?}");
 
-    let backends = benchmark_request.backends()?;
-    let profiles = benchmark_request.profiles()?;
+    let backends = request.backends()?;
+    let profiles = request.profiles()?;
     // Prevent the error from spamming the logs
     let mut has_emitted_parent_sha_error = false;
 
@@ -245,32 +251,36 @@ pub async fn enqueue_benchmark_request(
                     // If the parent job has been deleted from the database
                     // but was already benchmarked then the collector will ignore
                     // it as it will see it already has results.
-                    if let Some(parent_sha) = benchmark_request.parent_sha() {
-                        let (is_foreign_key_violation, result) = tx
-                            .conn()
-                            .enqueue_parent_benchmark_job(
-                                parent_sha,
-                                target,
-                                backend,
-                                profile,
-                                benchmark_set as u32,
-                            )
-                            .await;
-
-                        // At some point in time the parent_sha may not refer
-                        // to a `benchmark_request` and we want to be able to
-                        // see that error.
-                        if let Err(e) = result {
-                            if is_foreign_key_violation && !has_emitted_parent_sha_error {
-                                log::error!("Failed to create job for parent sha {e:?}");
-                                has_emitted_parent_sha_error = true;
-                            } else if has_emitted_parent_sha_error && is_foreign_key_violation {
-                                continue;
-                            } else {
-                                return Err(e);
-                            }
-                        }
-                    }
+
+                    // Do not enqueue parent jobs to allow parallel execution with the old system
+                    // If the parent artifact wouldn't be benchmarked yet, we would benchmark the
+                    // parent with the new system.
+                    // if let Some(parent_sha) = request.parent_sha() {
+                    //     let (is_foreign_key_violation, result) = tx
+                    //         .conn()
+                    //         .enqueue_parent_benchmark_job(
+                    //             parent_sha,
+                    //             target,
+                    //             backend,
+                    //             profile,
+                    //             benchmark_set as u32,
+                    //         )
+                    //         .await;
+                    //
+                    //     // At some point in time the parent_sha may not refer
+                    //     // to a `benchmark_request` and we want to be able to
+                    //     // see that error.
+                    //     if let Err(e) = result {
+                    //         if is_foreign_key_violation && !has_emitted_parent_sha_error {
+                    //             log::error!("Failed to create job for parent sha {e:?}");
+                    //             has_emitted_parent_sha_error = true;
+                    //         } else if has_emitted_parent_sha_error && is_foreign_key_violation {
+                    //             continue;
+                    //         } else {
+                    //             return Err(e);
+                    //         }
+                    //     }
+                    // }
                 }
             }
         }
@@ -294,12 +304,15 @@ async fn process_benchmark_requests(
 ) -> anyhow::Result<Vec<BenchmarkRequest>> {
     let queue = build_queue(conn).await?;
 
+    log::debug!("Current queue: {queue:?}");
+
     let mut completed = vec![];
     for request in queue {
         match request.status() {
             BenchmarkRequestStatus::InProgress => {
                 let tag = request.tag().expect("In progress request without a tag");
                 if conn.maybe_mark_benchmark_request_as_completed(tag).await? {
+                    log::info!("Request {tag} marked as completed");
                     completed.push(request);
                     continue;
                 }
@@ -318,8 +331,9 @@ async fn process_benchmark_requests(
     Ok(completed)
 }
 
-/// For queueing jobs, add the jobs you want to queue to this function
-async fn cron_enqueue_jobs(ctxt: &SiteCtxt) -> anyhow::Result<()> {
+/// Creates new benchmark requests, enqueues jobs for ready benchmark requests and
+/// finishes completed benchmark requests.
+async fn perform_queue_tick(ctxt: &SiteCtxt) -> anyhow::Result<()> {
     let mut conn = ctxt.conn().await;
 
     let index = ctxt.known_benchmark_requests.load();
@@ -394,7 +408,10 @@ async fn cron_enqueue_jobs(ctxt: &SiteCtxt) -> anyhow::Result<()> {
 }
 
 /// Entry point for the cron job that manages the benchmark request and job queue.
-pub async fn cron_main(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, run_interval: Duration) {
+pub async fn create_queue_process(
+    site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>,
+    run_interval: Duration,
+) {
     let mut interval = time::interval(run_interval);
     interval.set_missed_tick_behavior(MissedTickBehavior::Delay);
 
@@ -405,7 +422,7 @@ pub async fn cron_main(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, run_interv
             let guard = ctxt.read();
             guard.as_ref().cloned()
         } {
-            match cron_enqueue_jobs(&ctxt_clone).await {
+            match perform_queue_tick(&ctxt_clone).await {
                 Ok(_) => log::info!("Cron job finished"),
                 Err(e) => log::error!("Cron job failed to execute: {e:?}"),
             }
diff --git a/site/src/main.rs b/site/src/main.rs
@@ -1,6 +1,6 @@
 use futures::future::FutureExt;
 use parking_lot::RwLock;
-use site::job_queue::{cron_main, run_new_queue};
+use site::job_queue::{create_queue_process, run_new_queue};
 use site::load;
 use std::env;
 use std::sync::Arc;
@@ -61,7 +61,7 @@ async fn main() {
 
     if run_new_queue() {
         task::spawn(async move {
-            cron_main(
+            create_queue_process(
                 ctxt.clone(),
                 Duration::from_secs(queue_update_interval_seconds),
             )
diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs
@@ -3,7 +3,7 @@ use crate::github::{
     client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup,
     COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL,
 };
-use crate::job_queue::{run_new_queue, should_use_new_system};
+use crate::job_queue::should_use_new_system;
 use crate::load::SiteCtxt;
 
 use database::BenchmarkRequest;
