diff --git a/Cargo.lock b/Cargo.lock index 1d74ebec9..13949164a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -279,6 +279,7 @@ dependencies = [ "serde_json", "slog", "tokio", + "uuid", ] [[package]] @@ -1180,6 +1181,7 @@ dependencies = [ "tokio", "tokio-rustls 0.26.4", "url", + "uuid", ] [[package]] diff --git a/lib/api_projects/src/reports.rs b/lib/api_projects/src/reports.rs index fbc462be5..94f3b63ee 100644 --- a/lib/api_projects/src/reports.rs +++ b/lib/api_projects/src/reports.rs @@ -308,6 +308,7 @@ async fn post_inner( )?; let new_run_report = NewRunReport { report: json_report, + idempotency_key: None, #[cfg(feature = "plus")] is_claimed: true, #[cfg(feature = "plus")] diff --git a/lib/api_run/Cargo.toml b/lib/api_run/Cargo.toml index a4bc98954..966a690fe 100644 --- a/lib/api_run/Cargo.toml +++ b/lib/api_run/Cargo.toml @@ -29,6 +29,7 @@ diesel.workspace = true http.workspace = true serde_json.workspace = true tokio = { workspace = true, features = ["rt-multi-thread", "macros"] } +uuid.workspace = true [lints] workspace = true diff --git a/lib/api_run/src/lib.rs b/lib/api_run/src/lib.rs index 1dc9954cf..0266b466b 100644 --- a/lib/api_run/src/lib.rs +++ b/lib/api_run/src/lib.rs @@ -8,6 +8,8 @@ use http as _; use serde_json as _; #[cfg(test)] use tokio as _; +#[cfg(test)] +use uuid as _; mod run; diff --git a/lib/api_run/src/run.rs b/lib/api_run/src/run.rs index 3598b0545..e48d275cc 100644 --- a/lib/api_run/src/run.rs +++ b/lib/api_run/src/run.rs @@ -75,7 +75,7 @@ async fn post_inner( context: &ApiContext, public_user: &PublicUser, #[cfg(feature = "plus")] headers: &http::HeaderMap, - #[cfg_attr(not(feature = "plus"), expect(unused_mut))] mut json_run: JsonNewRun, + mut json_run: JsonNewRun, ) -> Result { match public_user { PublicUser::Public(remote_ip) => { @@ -165,8 +165,11 @@ async fn post_inner( slog::info!(log, "New run requested"; "project" => ?query_project, "run" => ?json_run); + let idempotency_key = json_run.idempotency_key.take(); + let new_run_report = NewRunReport { report: json_run.into(), + idempotency_key, #[cfg(feature = "plus")] is_claimed, #[cfg(feature = "plus")] diff --git a/lib/api_run/tests/run.rs b/lib/api_run/tests/run.rs index 86a413f3e..47274f967 100644 --- a/lib/api_run/tests/run.rs +++ b/lib/api_run/tests/run.rs @@ -1672,3 +1672,178 @@ async fn run_post_with_job_nonexistent_tag_fails() { // OCI digest resolution should fail for nonexistent tag assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } + +// POST /v0/run - idempotency key returns same report on retry +#[tokio::test] +async fn run_post_idempotency_key_returns_same_report() { + let server = TestServer::new().await; + let user = server.signup("Test User", "idempotency@example.com").await; + let org = server.create_org(&user, "Idempotency Org").await; + let project = server + .create_project(&user, &org, "Idempotency Project") + .await; + + let project_slug: &str = project.slug.as_ref(); + let idempotency_key = uuid::Uuid::new_v4().to_string(); + let body = serde_json::json!({ + "project": project_slug, + "idempotency_key": idempotency_key, + "branch": "main", + "testbed": "localhost", + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T00:01:00Z", + "results": [bmf_results().to_string()] + }); + + // First submission + let resp1 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body) + .send() + .await + .expect("First request failed"); + assert_eq!(resp1.status(), StatusCode::CREATED); + let report1: JsonReport = resp1.json().await.expect("Failed to parse first response"); + + // Second submission with same idempotency key returns same report + let resp2 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body) + .send() + .await + .expect("Second request failed"); + assert_eq!(resp2.status(), StatusCode::CREATED); + let report2: JsonReport = resp2.json().await.expect("Failed to parse second response"); + + assert_eq!(report1.uuid, report2.uuid); +} + +// POST /v0/run - different idempotency keys create different reports +#[tokio::test] +async fn run_post_different_idempotency_keys_create_different_reports() { + let server = TestServer::new().await; + let user = server + .signup("Test User", "idempotency_diff@example.com") + .await; + let org = server.create_org(&user, "Idempotency Diff Org").await; + let project = server + .create_project(&user, &org, "Idempotency Diff Project") + .await; + + let project_slug: &str = project.slug.as_ref(); + let bmf = bmf_results().to_string(); + + let body1 = serde_json::json!({ + "project": project_slug, + "idempotency_key": uuid::Uuid::new_v4().to_string(), + "branch": "main", + "testbed": "localhost", + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T00:01:00Z", + "results": [bmf] + }); + + let body2 = serde_json::json!({ + "project": project_slug, + "idempotency_key": uuid::Uuid::new_v4().to_string(), + "branch": "main", + "testbed": "localhost", + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T00:01:00Z", + "results": [bmf] + }); + + let resp1 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body1) + .send() + .await + .expect("First request failed"); + assert_eq!(resp1.status(), StatusCode::CREATED); + let report1: JsonReport = resp1.json().await.expect("Failed to parse first response"); + + let resp2 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body2) + .send() + .await + .expect("Second request failed"); + assert_eq!(resp2.status(), StatusCode::CREATED); + let report2: JsonReport = resp2.json().await.expect("Failed to parse second response"); + + assert_ne!(report1.uuid, report2.uuid); +} + +// POST /v0/run - no idempotency key always creates new reports +#[tokio::test] +async fn run_post_no_idempotency_key_creates_new_reports() { + let server = TestServer::new().await; + let user = server + .signup("Test User", "no_idempotency@example.com") + .await; + let org = server.create_org(&user, "No Idempotency Org").await; + let project = server + .create_project(&user, &org, "No Idempotency Project") + .await; + + let project_slug: &str = project.slug.as_ref(); + let body = serde_json::json!({ + "project": project_slug, + "branch": "main", + "testbed": "localhost", + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T00:01:00Z", + "results": [bmf_results().to_string()] + }); + + let resp1 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body) + .send() + .await + .expect("First request failed"); + assert_eq!(resp1.status(), StatusCode::CREATED); + let report1: JsonReport = resp1.json().await.expect("Failed to parse first response"); + + let resp2 = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&body) + .send() + .await + .expect("Second request failed"); + assert_eq!(resp2.status(), StatusCode::CREATED); + let report2: JsonReport = resp2.json().await.expect("Failed to parse second response"); + + // Without idempotency key, each submission creates a new report + assert_ne!(report1.uuid, report2.uuid); +} diff --git a/lib/bencher_json/src/project/report.rs b/lib/bencher_json/src/project/report.rs index 7832b1997..8b15ec41c 100644 --- a/lib/bencher_json/src/project/report.rs +++ b/lib/bencher_json/src/project/report.rs @@ -16,6 +16,7 @@ use crate::{ use super::{branch::JsonUpdateStartPoint, threshold::JsonThresholdModel}; crate::typed_uuid::typed_uuid!(ReportUuid); +crate::typed_uuid::typed_uuid!(ReportIdempotencyKey); #[derive(Debug, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(JsonSchema))] diff --git a/lib/bencher_json/src/run.rs b/lib/bencher_json/src/run.rs index 80273f112..18bd616ef 100644 --- a/lib/bencher_json/src/run.rs +++ b/lib/bencher_json/src/run.rs @@ -8,7 +8,7 @@ use crate::{ BranchNameId, ProjectResourceId, TestbedNameId, project::{ branch::JsonUpdateStartPoint, - report::{JsonReportSettings, JsonReportThresholds}, + report::{JsonReportSettings, JsonReportThresholds, ReportIdempotencyKey}, }, }; @@ -27,6 +27,11 @@ pub struct JsonNewRun { /// Project UUID or slug. /// If the project is not provided or does not exist, it will be created. pub project: Option, + /// Optional idempotency key for deduplicating run submissions. + /// If provided, a duplicate submission with the same key within the same project + /// will return the existing report instead of creating a new one. + #[serde(skip_serializing_if = "Option::is_none")] + pub idempotency_key: Option, /// Branch UUID, slug, or name. /// If the branch is not provided or does not exist, it will be created. pub branch: Option, @@ -78,6 +83,7 @@ impl From for JsonNewReport { fn from(run: JsonNewRun) -> Self { let JsonNewRun { project: _, + idempotency_key: _, branch, hash, start_point, diff --git a/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/down.sql b/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/down.sql new file mode 100644 index 000000000..fca29ac59 --- /dev/null +++ b/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/down.sql @@ -0,0 +1,45 @@ +PRAGMA foreign_keys = off; + +DROP INDEX IF EXISTS index_report_idempotency_key; + +CREATE TABLE down_report ( + id INTEGER PRIMARY KEY NOT NULL, + uuid TEXT NOT NULL UNIQUE, + user_id INTEGER, + project_id INTEGER NOT NULL, + head_id INTEGER NOT NULL, + version_id INTEGER NOT NULL, + testbed_id INTEGER NOT NULL, + spec_id INTEGER, + adapter INTEGER NOT NULL, + start_time BIGINT NOT NULL, + end_time BIGINT NOT NULL, + created BIGINT NOT NULL, + FOREIGN KEY (user_id) REFERENCES user (id), + FOREIGN KEY (project_id) REFERENCES project (id) ON DELETE CASCADE, + FOREIGN KEY (head_id) REFERENCES head (id), + FOREIGN KEY (version_id) REFERENCES version (id), + FOREIGN KEY (testbed_id) REFERENCES testbed (id), + FOREIGN KEY (spec_id) REFERENCES spec (id) ON DELETE SET NULL +); + +INSERT INTO down_report( + id, uuid, user_id, project_id, head_id, + version_id, testbed_id, spec_id, adapter, start_time, end_time, created + ) +SELECT id, uuid, user_id, project_id, head_id, + version_id, testbed_id, spec_id, adapter, start_time, end_time, created +FROM report; + +DROP TABLE report; +ALTER TABLE down_report RENAME TO report; + +CREATE INDEX index_report_testbed_end_time ON report(testbed_id, end_time); +CREATE INDEX index_report_version ON report(version_id, end_time); +CREATE INDEX index_report_project_end_time ON report(project_id, end_time); +CREATE INDEX index_report_project_created ON report(project_id, created); +CREATE INDEX index_report_version_testbed ON report(version_id, testbed_id); +CREATE INDEX index_report_spec ON report(spec_id); +CREATE INDEX index_report_head ON report(head_id); + +PRAGMA foreign_keys = on; diff --git a/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/up.sql b/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/up.sql new file mode 100644 index 000000000..c808deecc --- /dev/null +++ b/lib/bencher_schema/migrations/2026-03-31-120000_report_idempotency_key/up.sql @@ -0,0 +1,49 @@ +PRAGMA foreign_keys = off; + +CREATE TABLE up_report ( + id INTEGER PRIMARY KEY NOT NULL, + uuid TEXT NOT NULL UNIQUE, + idempotency_key TEXT, + user_id INTEGER, + project_id INTEGER NOT NULL, + head_id INTEGER NOT NULL, + version_id INTEGER NOT NULL, + testbed_id INTEGER NOT NULL, + spec_id INTEGER, + adapter INTEGER NOT NULL, + start_time BIGINT NOT NULL, + end_time BIGINT NOT NULL, + created BIGINT NOT NULL, + FOREIGN KEY (user_id) REFERENCES user (id), + FOREIGN KEY (project_id) REFERENCES project (id) ON DELETE CASCADE, + FOREIGN KEY (head_id) REFERENCES head (id), + FOREIGN KEY (version_id) REFERENCES version (id), + FOREIGN KEY (testbed_id) REFERENCES testbed (id), + FOREIGN KEY (spec_id) REFERENCES spec (id) ON DELETE SET NULL +); + +INSERT INTO up_report( + id, uuid, idempotency_key, user_id, project_id, head_id, + version_id, testbed_id, spec_id, adapter, start_time, end_time, created + ) +SELECT id, uuid, null, user_id, project_id, head_id, + version_id, testbed_id, spec_id, adapter, start_time, end_time, created +FROM report; + +DROP TABLE report; +ALTER TABLE up_report RENAME TO report; + +-- Recreate all existing indexes +CREATE INDEX index_report_testbed_end_time ON report(testbed_id, end_time); +CREATE INDEX index_report_version ON report(version_id, end_time); +CREATE INDEX index_report_project_end_time ON report(project_id, end_time); +CREATE INDEX index_report_project_created ON report(project_id, created); +CREATE INDEX index_report_version_testbed ON report(version_id, testbed_id); +CREATE INDEX index_report_spec ON report(spec_id); +CREATE INDEX index_report_head ON report(head_id); +-- New partial unique index for idempotency +CREATE UNIQUE INDEX index_report_idempotency_key + ON report(project_id, idempotency_key) + WHERE idempotency_key IS NOT NULL; + +PRAGMA foreign_keys = on; diff --git a/lib/bencher_schema/src/model/project/report/mod.rs b/lib/bencher_schema/src/model/project/report/mod.rs index c043b2654..a4bf0a53f 100644 --- a/lib/bencher_schema/src/model/project/report/mod.rs +++ b/lib/bencher_schema/src/model/project/report/mod.rs @@ -4,10 +4,9 @@ use bencher_json::{ DateTime, JsonNewReport, JsonReport, ReportUuid, project::report::{ Adapter, Iteration, JsonReportAlerts, JsonReportMeasure, JsonReportResult, - JsonReportResults, JsonReportSettings, + JsonReportResults, JsonReportSettings, ReportIdempotencyKey, }, }; -#[cfg(feature = "plus")] use diesel::OptionalExtension as _; use diesel::{ Connection as _, ExpressionMethods as _, NullableExpressionMethods as _, QueryDsl as _, @@ -50,6 +49,7 @@ use crate::{ /// Encapsulates all context from a run request for report creation. pub struct NewRunReport { pub report: JsonNewReport, + pub idempotency_key: Option, #[cfg(feature = "plus")] pub is_claimed: bool, #[cfg(feature = "plus")] @@ -96,6 +96,7 @@ crate::macros::typed_id::typed_id!(ReportId); pub struct QueryReport { pub id: ReportId, pub uuid: ReportUuid, + pub idempotency_key: Option, pub user_id: Option, pub project_id: ProjectId, pub head_id: HeadId, @@ -144,6 +145,7 @@ impl QueryReport { let NewRunReport { report: mut json_report, + idempotency_key, #[cfg(feature = "plus")] is_claimed, #[cfg(feature = "plus")] @@ -154,6 +156,15 @@ impl QueryReport { job: new_run_job, } = new_run_report; + // Idempotency check: if a key is provided, look for an existing report + if let Some(existing) = Self::check_idempotency( + public_conn!(context, public_user), + project_id, + idempotency_key, + )? { + return existing.into_json(log, public_conn!(context, public_user)); + } + #[cfg(all(feature = "plus", not(feature = "otel")))] let _ = is_claimed; #[cfg(all(feature = "plus", feature = "otel"))] @@ -270,6 +281,7 @@ impl QueryReport { // Create a new report and add it to the database let insert_report = InsertReport::from_json( + idempotency_key, public_user.user_id(), project_id, head_id, @@ -354,6 +366,29 @@ impl QueryReport { .await } + /// If an idempotency key is provided, check for an existing report with the same key. + fn check_idempotency( + conn: &mut DbConnection, + project_id: ProjectId, + idempotency_key: Option, + ) -> Result, HttpError> { + let Some(idempotency_key) = idempotency_key else { + return Ok(None); + }; + schema::report::table + .filter(schema::report::project_id.eq(project_id)) + .filter(schema::report::idempotency_key.eq(idempotency_key)) + .first::(conn) + .optional() + .map_err(|e| { + issue_error( + "Failed to check idempotency key", + "Failed to check report idempotency key", + e, + ) + }) + } + async fn finish_create( self, log: &Logger, @@ -378,6 +413,7 @@ impl QueryReport { let Self { id, uuid, + idempotency_key: _, user_id, project_id, head_id, @@ -711,6 +747,7 @@ fn get_report_alerts( #[diesel(table_name = report_table)] pub struct InsertReport { pub uuid: ReportUuid, + pub idempotency_key: Option, pub user_id: Option, pub project_id: ProjectId, pub head_id: HeadId, @@ -729,6 +766,7 @@ impl InsertReport { #[expect(clippy::too_many_arguments, reason = "report has many dimensions")] pub fn from_json( + idempotency_key: Option, user_id: Option, project_id: ProjectId, head_id: HeadId, @@ -741,6 +779,7 @@ impl InsertReport { ) -> Self { Self { uuid: ReportUuid::new(), + idempotency_key, user_id, project_id, head_id, diff --git a/lib/bencher_schema/src/schema.rs b/lib/bencher_schema/src/schema.rs index 33363297b..a9fe4eda0 100644 --- a/lib/bencher_schema/src/schema.rs +++ b/lib/bencher_schema/src/schema.rs @@ -265,6 +265,7 @@ diesel::table! { report (id) { id -> Integer, uuid -> Text, + idempotency_key -> Nullable, user_id -> Nullable, project_id -> Integer, head_id -> Integer, @@ -421,9 +422,9 @@ diesel::joinable!(branch -> project (project_id)); diesel::joinable!(head_version -> version (version_id)); diesel::joinable!(job -> organization (organization_id)); diesel::joinable!(job -> report (report_id)); -diesel::joinable!(job_duration_by_report -> report (report_id)); diesel::joinable!(job -> runner (runner_id)); diesel::joinable!(job -> spec (spec_id)); +diesel::joinable!(job_duration_by_report -> report (report_id)); diesel::joinable!(measure -> project (project_id)); diesel::joinable!(metric -> measure (measure_id)); diesel::joinable!(metric -> report_benchmark (report_benchmark_id)); diff --git a/services/api/openapi.json b/services/api/openapi.json index 506e5c012..61378251e 100644 --- a/services/api/openapi.json +++ b/services/api/openapi.json @@ -12715,6 +12715,15 @@ } ] }, + "idempotency_key": { + "nullable": true, + "description": "Optional idempotency key for deduplicating run submissions. If provided, a duplicate submission with the same key within the same project will return the existing report instead of creating a new one.", + "allOf": [ + { + "$ref": "#/components/schemas/ReportIdempotencyKey" + } + ] + }, "job": { "nullable": true, "description": "Runner job configuration. When present, the run is executed on a remote bare metal runner instead of locally.", @@ -16747,6 +16756,10 @@ } ] }, + "ReportIdempotencyKey": { + "type": "string", + "format": "uuid" + }, "ReportUuid": { "type": "string", "format": "uuid" diff --git a/services/cli/Cargo.toml b/services/cli/Cargo.toml index 38737a6d6..1556572d9 100644 --- a/services/cli/Cargo.toml +++ b/services/cli/Cargo.toml @@ -38,6 +38,7 @@ thiserror.workspace = true tokio = { workspace = true, features = ["macros", "process", "rt", "signal"] } tokio-rustls.workspace = true url.workspace = true +uuid.workspace = true [lints] workspace = true diff --git a/services/cli/src/bencher/sub/run/mod.rs b/services/cli/src/bencher/sub/run/mod.rs index 0fb92a907..47de68b5d 100644 --- a/services/cli/src/bencher/sub/run/mod.rs +++ b/services/cli/src/bencher/sub/run/mod.rs @@ -280,6 +280,7 @@ impl Run { let (branch, hash, start_point) = self.branch.clone().into(); Ok(Some(JsonNewRun { project: self.project.clone().map(Into::into), + idempotency_key: Some(uuid::Uuid::new_v4().into()), branch, hash, start_point, @@ -313,6 +314,7 @@ impl Run { let (branch, hash, start_point) = self.branch.clone().into(); JsonNewRun { project: self.project.clone().map(Into::into), + idempotency_key: Some(uuid::Uuid::new_v4().into()), branch, hash, start_point, diff --git a/services/console/src/chunks/docs-reference/changelog/en/changelog.mdx b/services/console/src/chunks/docs-reference/changelog/en/changelog.mdx index 475c71a50..073953f8d 100644 --- a/services/console/src/chunks/docs-reference/changelog/en/changelog.mdx +++ b/services/console/src/chunks/docs-reference/changelog/en/changelog.mdx @@ -1,5 +1,6 @@ ## Pending `v0.6.1` - Buffer OCI upload network frames into configurable S3 chunks (`registry.data_store.chunk_size`, default 5 MB) to reduce S3 operations during Docker push +- Add `idempotency_key` to `bencher run` to deduplicate retried submissions ## `v0.6.0` - **BREAKING CHANGE** Update Bencher Metric Format (BMF) to accept UUID, slug, or name for the Benchmark identifier