From e7af397e849cf4be70634ed5b7fcf4c99758b991 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 20 Jul 2026 08:04:47 +0000 Subject: [PATCH] fix(alerts): build the gate index concurrently, void marks on cancellation The index on v2_job_queue ran as an ordinary CREATE INDEX inside the migration transaction, holding the locks taken by the preceding ALTERs while scanning a live, actively written queue. Register the migration in OVERRIDDEN_MIGRATIONS so it is rewritten to CREATE INDEX CONCURRENTLY and executed as top-level statements, matching the other queue indexes. Soft cancellation also leaves a stale gate mark on a job the pull path will hand straight to a worker, so a cancelled job now counts as ungated whatever mark it carries. Emitter and limiter changes live in windmill-ee-private (see ee-repo-ref.txt bump). --- ...6135d8451abe7b9f97b668998e456a1af639.json} | 4 +-- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/src/db.rs | 3 ++ .../tests/jobs_waiting_alerts.rs | 35 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) rename backend/.sqlx/{query-428bd28304df56528604da2a5f83f0e989cb96acbd2eaff99e29110d96625f64.json => query-37bdc3c62326be091b0b09fac5dd6135d8451abe7b9f97b668998e456a1af639.json} (73%) diff --git a/backend/.sqlx/query-428bd28304df56528604da2a5f83f0e989cb96acbd2eaff99e29110d96625f64.json b/backend/.sqlx/query-37bdc3c62326be091b0b09fac5dd6135d8451abe7b9f97b668998e456a1af639.json similarity index 73% rename from backend/.sqlx/query-428bd28304df56528604da2a5f83f0e989cb96acbd2eaff99e29110d96625f64.json rename to backend/.sqlx/query-37bdc3c62326be091b0b09fac5dd6135d8451abe7b9f97b668998e456a1af639.json index 19ee787d87..3981121d62 100644 --- a/backend/.sqlx/query-428bd28304df56528604da2a5f83f0e989cb96acbd2eaff99e29110d96625f64.json +++ b/backend/.sqlx/query-37bdc3c62326be091b0b09fac5dd6135d8451abe7b9f97b668998e456a1af639.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT COUNT(*) FILTER (WHERE overdue AND gated IS NOT TRUE) AS ungated_count,\n COUNT(*) FILTER (WHERE overdue AND gated) AS gated_count,\n MIN(scheduled_for) FILTER (WHERE overdue AND gated IS NOT TRUE)\n AS oldest_ungated\n FROM (\n SELECT scheduled_for,\n scheduled_for <= NOW() - $2::interval AS overdue,\n concurrency_gate_id = ANY($3::text[]) AS gated\n FROM v2_job_queue\n WHERE tag = $1 AND running = false\n ) q\n ", + "query": "\n SELECT COUNT(*) FILTER (WHERE overdue AND gated IS NOT TRUE) AS ungated_count,\n COUNT(*) FILTER (WHERE overdue AND gated) AS gated_count,\n MIN(scheduled_for) FILTER (WHERE overdue AND gated IS NOT TRUE)\n AS oldest_ungated\n FROM (\n SELECT scheduled_for,\n scheduled_for <= NOW() - $2::interval AS overdue,\n canceled_by IS NULL\n AND concurrency_gate_id = ANY($3::text[]) AS gated\n FROM v2_job_queue\n WHERE tag = $1 AND running = false\n ) q\n ", "describe": { "columns": [ { @@ -32,5 +32,5 @@ null ] }, - "hash": "428bd28304df56528604da2a5f83f0e989cb96acbd2eaff99e29110d96625f64" + "hash": "37bdc3c62326be091b0b09fac5dd6135d8451abe7b9f97b668998e456a1af639" } diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 2cbf91cdc3..530732c02e 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -d3445d03c1f103f7fc5432a7d898a5ae8db728ee \ No newline at end of file +e1e73f4da9349b2aa58d4e113ffae2cb3dc6646c \ No newline at end of file diff --git a/backend/windmill-api/src/db.rs b/backend/windmill-api/src/db.rs index c564548b42..8d32214261 100644 --- a/backend/windmill-api/src/db.rs +++ b/backend/windmill-api/src/db.rs @@ -93,6 +93,9 @@ lazy_static::lazy_static! { (20260710073406, include_str!( "../../migrations/20260710073406_index_v2_job_parent_job.up.sql" ).replace("CREATE INDEX", "CREATE INDEX CONCURRENTLY")), + (20260720055519, include_str!( + "../../migrations/20260720055519_concurrency_gated_flag.up.sql" + ).replace("CREATE INDEX", "CREATE INDEX CONCURRENTLY")), ].into_iter().collect(); } diff --git a/backend/windmill-common/tests/jobs_waiting_alerts.rs b/backend/windmill-common/tests/jobs_waiting_alerts.rs index 2b176bcedf..a70a696593 100644 --- a/backend/windmill-common/tests/jobs_waiting_alerts.rs +++ b/backend/windmill-common/tests/jobs_waiting_alerts.rs @@ -63,6 +63,20 @@ mod tests { queue_jobs_on_tag(db, TAG, n, gate).await } + /// Soft-cancelled jobs keep whatever mark they carried, but the pull path + /// skips the limiter for them, so they are waiting for a worker. + async fn queue_canceled_jobs(db: &Pool, n: usize, gate: Option<(&str, i64)>) { + queue_jobs_on_tag(db, TAG, n, gate).await; + sqlx::query!( + "UPDATE v2_job_queue SET canceled_by = 'canceller' + WHERE tag = $1 AND canceled_by IS NULL", + TAG, + ) + .execute(db) + .await + .expect("cancel jobs"); + } + async fn queue_jobs_on_tag( db: &Pool, tag: &str, @@ -220,4 +234,25 @@ mod tests { "a gate still being refreshed on another tag must keep holding this tag's jobs" ); } + + /// Cancellation makes the limiter inapplicable, so a mark left from before it + /// was cancelled no longer describes anything. If its gate happens to stay + /// live elsewhere, keying off the mark alone excludes a job that is squarely + /// waiting for a worker. + #[ignore = "requires database setup - run with --ignored flag"] + #[sqlx::test(migrations = "../migrations")] + async fn canceled_jobs_are_not_held_by_a_live_gate(db: Pool) { + free_the_lock(&db).await; + configure_alert(&db, 100).await; + queue_canceled_jobs(&db, 200, Some((BUSY_GATE, 1))).await; + + jobs_waiting_alerts(&db).await; + + let messages = alert_messages(&db).await; + assert_eq!( + messages.len(), + 1, + "cancelled jobs bypass the limiter and must still alert, got {messages:?}" + ); + } }