Merge pull request #2201 from Kobzol/cron-job-batch

Refactor benchmark requests

Author: Kobzol

database/schema.md

@@ -258,3 +258,41 @@ aid         benchmark   error
 ----------  ---         -----
 1           syn-1.0.89  Failed to compile...
 ```
+
+
+## New benchmarking design
+We are currently implementing a new design for dispatching benchmarks to collector(s) and storing
+them in the database. It will support new use-cases, like backfilling of new benchmarks into a parent
+commit and primarily benchmarking with multiple collectors (and multiple hardware architectures) in
+parallel.
+
+The tables below are a part of the new scheme.
+
+### benchmark_request
+
+Represents a single request for performing a benchmark collection. Each request can be one of three types:
+
+* Master: benchmark a merged master commit
+* Release: benchmark a published stable or beta compiler toolchain
+* Try: benchmark a try build on a PR
+
+Columns:
+
+* **id** (`int`): Unique ID.
+* **tag** (`text`): Identifier of the compiler toolchain that should be benchmarked.
+  * Commit SHA for master/try requests or release name (e.g. `1.80.0`) for release requests.
+  * Can be `NULL` for try requests that were queued for a perf. run, but their compiler artifacts haven't been built yet.
+* **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`.
+* **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.
+* **completed_at** (`timestamptz`): Datetime when the request was completed.
+* **status** (`text NOT NULL`): Current status of the benchmark request.
+  * `waiting-for-artifacts`: A try request waiting for compiler artifacts.
+  * `artifacts-ready`: Request that has compiler artifacts ready and can be benchmarked.
+  * `in-progress`: Request that is currently being benchmarked.
+  * `completed`: Completed request.
+* **backends** (`text NOT NULL`): Comma-separated list of codegen backends to benchmark. If empty, the default set of codegen backends will be benchmarked.
+* **profiles** (`text NOT NULL`): Comma-separated list of profiles to benchmark. If empty, the default set of profiles will be benchmarked.

database/src/lib.rs

@@ -1,6 +1,6 @@
 use chrono::offset::TimeZone;
 use chrono::{DateTime, Utc};
-use hashbrown::HashMap;
+use hashbrown::{HashMap, HashSet};
 use intern::intern;
 use serde::{Deserialize, Serialize};
 use std::fmt;
@@ -309,6 +309,7 @@ impl Scenario {
     }
 }
 
+use anyhow::anyhow;
 use std::cmp::Ordering;
 use std::str::FromStr;
 
@@ -797,53 +798,64 @@ pub struct ArtifactCollection {
     pub end_time: DateTime<Utc>,
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Copy, Clone, PartialEq)]
 pub enum BenchmarkRequestStatus {
     WaitingForArtifacts,
     ArtifactsReady,
     InProgress,
-    Completed,
+    Completed { completed_at: DateTime<Utc> },
 }
 
+const BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR: &str = "waiting_for_artifacts";
+const BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR: &str = "artifacts_ready";
+const BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR: &str = "in_progress";
+const BENCHMARK_REQUEST_STATUS_COMPLETED_STR: &str = "completed";
+
 impl BenchmarkRequestStatus {
-    pub fn as_str(&self) -> &str {
+    pub(crate) fn as_str(&self) -> &str {
         match self {
-            BenchmarkRequestStatus::WaitingForArtifacts => "waiting_for_artifacts",
-            BenchmarkRequestStatus::ArtifactsReady => "artifacts_ready",
-            BenchmarkRequestStatus::InProgress => "in_progress",
-            BenchmarkRequestStatus::Completed => "completed",
+            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::Completed { .. } => BENCHMARK_REQUEST_STATUS_COMPLETED_STR,
         }
     }
-}
 
-impl fmt::Display for BenchmarkRequestStatus {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(self.as_str())
+    pub(crate) fn from_str_and_completion_date(
+        text: &str,
+        completion_date: Option<DateTime<Utc>>,
+    ) -> anyhow::Result<Self> {
+        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_COMPLETED_STR => Ok(Self::Completed {
+                completed_at: completion_date.ok_or_else(|| {
+                    anyhow!("No completion date for a completed BenchmarkRequestStatus")
+                })?,
+            }),
+            _ => Err(anyhow!("Unknown BenchmarkRequestStatus `{text}`")),
+        }
     }
-}
 
-impl<'a> tokio_postgres::types::FromSql<'a> for BenchmarkRequestStatus {
-    fn from_sql(
-        ty: &tokio_postgres::types::Type,
-        raw: &'a [u8],
-    ) -> Result<Self, Box<dyn std::error::Error + Sync + Send>> {
-        // Decode raw bytes into &str with Postgres' own text codec
-        let s: &str = <&str as tokio_postgres::types::FromSql>::from_sql(ty, raw)?;
-
-        match s {
-            x if x == Self::WaitingForArtifacts.as_str() => Ok(Self::WaitingForArtifacts),
-            x if x == Self::ArtifactsReady.as_str() => Ok(Self::ArtifactsReady),
-            x if x == Self::InProgress.as_str() => Ok(Self::InProgress),
-            x if x == Self::Completed.as_str() => Ok(Self::Completed),
-            other => Err(format!("unknown benchmark_request_status '{other}'").into()),
+    pub(crate) fn completed_at(&self) -> Option<DateTime<Utc>> {
+        match self {
+            Self::Completed { completed_at } => Some(*completed_at),
+            _ => None,
         }
     }
+}
 
-    fn accepts(ty: &tokio_postgres::types::Type) -> bool {
-        <&str as tokio_postgres::types::FromSql>::accepts(ty)
+impl fmt::Display for BenchmarkRequestStatus {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.as_str())
     }
 }
 
+const BENCHMARK_REQUEST_TRY_STR: &str = "try";
+const BENCHMARK_REQUEST_MASTER_STR: &str = "master";
+const BENCHMARK_REQUEST_RELEASE_STR: &str = "release";
+
 #[derive(Debug, Clone, PartialEq)]
 pub enum BenchmarkRequestType {
     /// A Try commit
@@ -865,86 +877,68 @@ pub enum BenchmarkRequestType {
 impl fmt::Display for BenchmarkRequestType {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            BenchmarkRequestType::Try { .. } => write!(f, "try"),
-            BenchmarkRequestType::Master { .. } => write!(f, "master"),
-            BenchmarkRequestType::Release { .. } => write!(f, "release"),
+            BenchmarkRequestType::Try { .. } => write!(f, "{BENCHMARK_REQUEST_TRY_STR}"),
+            BenchmarkRequestType::Master { .. } => write!(f, "{BENCHMARK_REQUEST_MASTER_STR}"),
+            BenchmarkRequestType::Release { .. } => write!(f, "{BENCHMARK_REQUEST_RELEASE_STR}"),
         }
     }
 }
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct BenchmarkRequest {
-    pub commit_type: BenchmarkRequestType,
-    pub created_at: DateTime<Utc>,
-    pub completed_at: Option<DateTime<Utc>>,
-    pub status: BenchmarkRequestStatus,
-    pub backends: String,
-    pub profiles: String,
+    commit_type: BenchmarkRequestType,
+    created_at: DateTime<Utc>,
+    status: BenchmarkRequestStatus,
+    backends: String,
+    profiles: String,
 }
 
 impl BenchmarkRequest {
-    pub fn create_release(
-        tag: &str,
-        created_at: DateTime<Utc>,
-        status: BenchmarkRequestStatus,
-        backends: &str,
-        profiles: &str,
-    ) -> Self {
+    /// Create a release benchmark request that is in the `ArtifactsReady` status.
+    pub fn create_release(tag: &str, created_at: DateTime<Utc>) -> Self {
         Self {
             commit_type: BenchmarkRequestType::Release {
                 tag: tag.to_string(),
             },
             created_at,
-            completed_at: None,
-            status,
-            backends: backends.to_string(),
-            profiles: profiles.to_string(),
+            status: BenchmarkRequestStatus::ArtifactsReady,
+            backends: String::new(),
+            profiles: String::new(),
         }
     }
 
-    pub fn create_try(
-        sha: Option<&str>,
-        parent_sha: Option<&str>,
+    /// Create a try request that is in the `WaitingForArtifacts` status.
+    pub fn create_try_without_artifacts(
         pr: u32,
         created_at: DateTime<Utc>,
-        status: BenchmarkRequestStatus,
         backends: &str,
         profiles: &str,
     ) -> Self {
         Self {
             commit_type: BenchmarkRequestType::Try {
                 pr,
-                sha: sha.map(|it| it.to_string()),
-                parent_sha: parent_sha.map(|it| it.to_string()),
+                sha: None,
+                parent_sha: None,
             },
             created_at,
-            completed_at: None,
-            status,
+            status: BenchmarkRequestStatus::WaitingForArtifacts,
             backends: backends.to_string(),
             profiles: profiles.to_string(),
         }
     }
 
-    pub fn create_master(
-        sha: &str,
-        parent_sha: &str,
-        pr: u32,
-        created_at: DateTime<Utc>,
-        status: BenchmarkRequestStatus,
-        backends: &str,
-        profiles: &str,
-    ) -> Self {
+    /// 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 {
         Self {
             commit_type: BenchmarkRequestType::Master {
                 pr,
                 sha: sha.to_string(),
                 parent_sha: parent_sha.to_string(),
             },
             created_at,
-            completed_at: None,
-            status,
-            backends: backends.to_string(),
-            profiles: profiles.to_string(),
+            status: BenchmarkRequestStatus::ArtifactsReady,
+            backends: String::new(),
+            profiles: String::new(),
         }
     }
 
@@ -974,4 +968,45 @@ impl BenchmarkRequest {
             BenchmarkRequestType::Release { .. } => None,
         }
     }
+
+    pub fn status(&self) -> BenchmarkRequestStatus {
+        self.status
+    }
+
+    pub fn created_at(&self) -> DateTime<Utc> {
+        self.created_at
+    }
+
+    pub fn is_master(&self) -> bool {
+        matches!(self.commit_type, BenchmarkRequestType::Master { .. })
+    }
+
+    pub fn is_try(&self) -> bool {
+        matches!(self.commit_type, BenchmarkRequestType::Try { .. })
+    }
+
+    pub fn is_release(&self) -> bool {
+        matches!(self.commit_type, BenchmarkRequestType::Release { .. })
+    }
+}
+
+/// Cached information about benchmark requests in the DB
+/// FIXME: only store non-try requests here
+pub struct BenchmarkRequestIndex {
+    /// Tags (SHA or release name) of all known benchmark requests
+    all: HashSet<String>,
+    /// Tags (SHA or release name) of all benchmark requests in the completed status
+    completed: HashSet<String>,
+}
+
+impl BenchmarkRequestIndex {
+    /// Do we already have a benchmark request for the passed `tag`?
+    pub fn contains_tag(&self, tag: &str) -> bool {
+        self.all.contains(tag)
+    }
+
+    /// Return tags of already completed benchmark requests.
+    pub fn completed_requests(&self) -> &HashSet<String> {
+        &self.completed
+    }
 }