From 64ed2352d9c0e7270e6a765c1b1ba6c635804fa5 Mon Sep 17 00:00:00 2001 From: Jamesbarford Date: Wed, 11 Jun 2025 14:54:45 +0100 Subject: [PATCH 1/6] Create new table for `collector_config`, configuration for the collectors --- database/schema.md | 13 ++++++++++++- database/src/pool/postgres.rs | 11 +++++++++++ database/src/pool/sqlite.rs | 12 ++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/database/schema.md b/database/schema.md index 6c8194d7d..bc0404ee0 100644 --- a/database/schema.md +++ b/database/schema.md @@ -259,7 +259,6 @@ 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 @@ -296,3 +295,15 @@ Columns: * `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. + +### collector_config + +Information about the collector; it's target architecture, when it was added, whether it is active and when it last had activity denoted by `last_heartbeat_at`. + +``` +sqlite> SELECT * FROM collector_config; + +id target date_added last_heartbeat_at benchmark_set is_active +--------- ------------------------- ------------- ---------------- --------- ------- +ea1f4e... aarch64-unknown-linux-gnu 2025-06-11... 2025-06-12 17... cea1bc... 0 +``` diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index b8079f5a7..b3a2ccf4f 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -309,6 +309,17 @@ static MIGRATIONS: &[&str] = &[ // Prevent multiple try commits without a `sha` and the same `pr` number // being added to the table r#"CREATE UNIQUE INDEX benchmark_request_pr_commit_type_idx ON benchmark_request (pr, commit_type) WHERE status != 'completed';"#, + r#"CREATE EXTENSION IF NOT EXISTS "uuid-ossp";"#, + r#" + CREATE TABLE IF NOT EXISTS collector_config ( + id UUID PRIMARY KEY, + target TEXT NOT NULL, + date_added TIMESTAMPTZ DEFAULT NOW() NOT NULL, + last_heartbeat_at TIMESTAMPTZ, + benchmark_set UUID NOT NULL, + is_active BOOLEAN DEFAULT FALSE NOT NULL + ); + "#, ]; #[async_trait::async_trait] diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index ecc28484f..89177b669 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -405,6 +405,18 @@ static MIGRATIONS: &[Migration] = &[ alter table pstat_series_with_target rename to pstat_series; "#, ), + Migration::without_foreign_key_constraints( + r#" + CREATE TABLE IF NOT EXISTS collector_config ( + id TEXT PRIMARY KEY, + target TEXT NOT NULL, + date_added TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, + last_heartbeat_at TIMESTAMP, + benchmark_set TEXT NOT NULL, + is_active BOOLEAN DEFAULT FALSE NOT NULL + ); + "#, + ), ]; #[async_trait::async_trait] From 41403501120e78f572a89afda2ab196725622d75 Mon Sep 17 00:00:00 2001 From: Jamesbarford Date: Wed, 11 Jun 2025 15:47:35 +0100 Subject: [PATCH 2/6] PR feedback; use INTEGER rather than UUID --- database/schema.md | 2 +- database/src/pool/postgres.rs | 6 +++--- database/src/pool/sqlite.rs | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/database/schema.md b/database/schema.md index bc0404ee0..3bd46dd59 100644 --- a/database/schema.md +++ b/database/schema.md @@ -305,5 +305,5 @@ sqlite> SELECT * FROM collector_config; id target date_added last_heartbeat_at benchmark_set is_active --------- ------------------------- ------------- ---------------- --------- ------- -ea1f4e... aarch64-unknown-linux-gnu 2025-06-11... 2025-06-12 17... cea1bc... 0 +1 aarch64-unknown-linux-gnu 2025-06-11... 2025-06-12 17... 2 0 ``` diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index b3a2ccf4f..190e799df 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -309,14 +309,14 @@ static MIGRATIONS: &[&str] = &[ // Prevent multiple try commits without a `sha` and the same `pr` number // being added to the table r#"CREATE UNIQUE INDEX benchmark_request_pr_commit_type_idx ON benchmark_request (pr, commit_type) WHERE status != 'completed';"#, - r#"CREATE EXTENSION IF NOT EXISTS "uuid-ossp";"#, r#" CREATE TABLE IF NOT EXISTS collector_config ( - id UUID PRIMARY KEY, + id SERIAL PRIMARY KEY, target TEXT NOT NULL, + name TEXT NOT NULL, date_added TIMESTAMPTZ DEFAULT NOW() NOT NULL, last_heartbeat_at TIMESTAMPTZ, - benchmark_set UUID NOT NULL, + benchmark_set INTEGER NOT NULL, is_active BOOLEAN DEFAULT FALSE NOT NULL ); "#, diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index 89177b669..c7eefcfb1 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -408,11 +408,12 @@ static MIGRATIONS: &[Migration] = &[ Migration::without_foreign_key_constraints( r#" CREATE TABLE IF NOT EXISTS collector_config ( - id TEXT PRIMARY KEY, + id INTEGER AUTO INCREMENT PRIMARY KEY, target TEXT NOT NULL, + name TEXT NOT NULL, date_added TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, last_heartbeat_at TIMESTAMP, - benchmark_set TEXT NOT NULL, + benchmark_set INTEGER NOT NULL, is_active BOOLEAN DEFAULT FALSE NOT NULL ); "#, From ef42ae21800dd1c67a4a8bd57a445bca186422e4 Mon Sep 17 00:00:00 2001 From: Jamesbarford Date: Thu, 12 Jun 2025 14:59:47 +0100 Subject: [PATCH 3/6] Pr feedback; add name to the schema doc and remove SQLite for the time being --- database/schema.md | 6 +++--- database/src/pool/sqlite.rs | 13 ------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/database/schema.md b/database/schema.md index 3bd46dd59..243c8477c 100644 --- a/database/schema.md +++ b/database/schema.md @@ -303,7 +303,7 @@ Information about the collector; it's target architecture, when it was added, wh ``` sqlite> SELECT * FROM collector_config; -id target date_added last_heartbeat_at benchmark_set is_active ---------- ------------------------- ------------- ---------------- --------- ------- -1 aarch64-unknown-linux-gnu 2025-06-11... 2025-06-12 17... 2 0 +id target name date_added last_heartbeat_at benchmark_set is_active +--------- ------------------------- ---- ------------- ---------------- --------- ------- +1 aarch64-unknown-linux-gnu foo 2025-06-11... 2025-06-12 17... 2 0 ``` diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index c7eefcfb1..ecc28484f 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -405,19 +405,6 @@ static MIGRATIONS: &[Migration] = &[ alter table pstat_series_with_target rename to pstat_series; "#, ), - Migration::without_foreign_key_constraints( - r#" - CREATE TABLE IF NOT EXISTS collector_config ( - id INTEGER AUTO INCREMENT PRIMARY KEY, - target TEXT NOT NULL, - name TEXT NOT NULL, - date_added TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, - last_heartbeat_at TIMESTAMP, - benchmark_set INTEGER NOT NULL, - is_active BOOLEAN DEFAULT FALSE NOT NULL - ); - "#, - ), ]; #[async_trait::async_trait] From e8b9121b8501e1b88a4442bddef115285a390e12 Mon Sep 17 00:00:00 2001 From: Jamesbarford Date: Thu, 19 Jun 2025 13:28:35 +0100 Subject: [PATCH 4/6] Add unique constraint on `(target, benchmark_set, is_active)` --- database/src/pool/postgres.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index 190e799df..9208f5e10 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -313,11 +313,16 @@ static MIGRATIONS: &[&str] = &[ CREATE TABLE IF NOT EXISTS collector_config ( id SERIAL PRIMARY KEY, target TEXT NOT NULL, - name TEXT NOT NULL, + name TEXT NOT NULL UNIQUE, date_added TIMESTAMPTZ DEFAULT NOW() NOT NULL, last_heartbeat_at TIMESTAMPTZ, benchmark_set INTEGER NOT NULL, - is_active BOOLEAN DEFAULT FALSE NOT NULL + is_active BOOLEAN DEFAULT FALSE NOT NULL, + + -- Given the current setup, we do not want 2 collectors that are active + -- with the same target using the same benchmark set. + CONSTRAINT collector_config_target_bench_active_uniq + UNIQUE (target, benchmark_set, is_active) ); "#, ]; From 055650d18acc7869d7032470417ad93f52c16b90 Mon Sep 17 00:00:00 2001 From: James Barford-Evans Date: Tue, 15 Jul 2025 08:26:20 +0100 Subject: [PATCH 5/6] Modernised the `collector_config` schema description --- database/schema.md | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/database/schema.md b/database/schema.md index 243c8477c..9ae0045cc 100644 --- a/database/schema.md +++ b/database/schema.md @@ -298,12 +298,18 @@ Columns: ### collector_config -Information about the collector; it's target architecture, when it was added, whether it is active and when it last had activity denoted by `last_heartbeat_at`. +Information about the collector; it's target architecture, when it was added, +whether it is active and when it last had activity denoted by `last_heartbeat_at`. -``` -sqlite> SELECT * FROM collector_config; +Columns: -id target name date_added last_heartbeat_at benchmark_set is_active ---------- ------------------------- ---- ------------- ---------------- --------- ------- -1 aarch64-unknown-linux-gnu foo 2025-06-11... 2025-06-12 17... 2 0 -``` +* **id** (`id`): A unique identifier for the collector. +* **target** (`text NOT NULL`): The ISA of the collector for example; `AArch64`. +* **name** (`text NOT NULL`): Unique name for the collector. +* **date_added** (`timestamptz NOT NULL`): When the collector was added +* **last_heartbeat_at** (`timestamptz`): When the collector last updated this + column, a way to test if the collector is still alive. +* **benchmark_set** (`int NOT NULL`): ID of the predefined benchmark suite to + execute. +* **is_active** (`boolean NOT NULL`): For controlling whether the collector is + active for use. Useful for adding/removing collectors. From 2e7ced8a578b90cd901e31e548ed8af84b4d9e4c Mon Sep 17 00:00:00 2001 From: James Barford-Evans Date: Thu, 17 Jul 2025 10:35:43 +0100 Subject: [PATCH 6/6] PR Feedback; update index and `schmea.md` --- database/schema.md | 2 +- database/src/pool/postgres.rs | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/database/schema.md b/database/schema.md index 9ae0045cc..3d8ab3b5b 100644 --- a/database/schema.md +++ b/database/schema.md @@ -304,7 +304,7 @@ whether it is active and when it last had activity denoted by `last_heartbeat_at Columns: * **id** (`id`): A unique identifier for the collector. -* **target** (`text NOT NULL`): The ISA of the collector for example; `AArch64`. +* **target** (`text NOT NULL`): The ISA of the collector for example; `aarch64-unknown-linux-gnu`. * **name** (`text NOT NULL`): Unique name for the collector. * **date_added** (`timestamptz NOT NULL`): When the collector was added * **last_heartbeat_at** (`timestamptz`): When the collector last updated this diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index 9208f5e10..21c0d3717 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -317,13 +317,12 @@ static MIGRATIONS: &[&str] = &[ date_added TIMESTAMPTZ DEFAULT NOW() NOT NULL, last_heartbeat_at TIMESTAMPTZ, benchmark_set INTEGER NOT NULL, - is_active BOOLEAN DEFAULT FALSE NOT NULL, - - -- Given the current setup, we do not want 2 collectors that are active - -- with the same target using the same benchmark set. - CONSTRAINT collector_config_target_bench_active_uniq - UNIQUE (target, benchmark_set, is_active) + is_active BOOLEAN DEFAULT FALSE NOT NULL ); + -- Given the current setup, we do not want 2 collectors that are active + -- with the same target using the same benchmark set. + CREATE UNIQUE INDEX collector_config_target_bench_active_uniq ON collector_config + (target, benchmark_set, is_active) WHERE is_active = TRUE; "#, ];