mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-28 16:02:47 +00:00
feat(queue): per-workspace fairness cap on the shared cloud worker pool (#9303)
* feat(queue): cloud-only per-workspace fairness cap on the shared worker pool
On `app.windmill.dev` the cluster runs a single default worker group, so a
single workspace flooding the queue can degrade quality of service for
everyone else. This adds an opt-in mechanism that caps any single workspace
at a configurable share of the shared worker pool when it has been
dominating cluster activity for more than a configurable window.
Detection signal counts both currently-running jobs and jobs completed in
the rolling window, so it catches workspaces hogging slots with long jobs
**and** workspaces spamming many tiny jobs (where no individual job's
started_at is old, but throughput share dominates).
Refresh is coordinated cluster-wide via a single UPDATE on
`background_task_state`: the `WHERE updated_at < now() - interval` predicate
combined with row-level locking means only one process per refresh cycle
actually runs the aggregation, regardless of fleet size. Every other
process gets the freshly written value in the same round trip via
`UNION ALL ... LIMIT 1`. Heavy aggregation rate stays at ~0.2-0.5 qps for
the whole cluster.
Pull queries are split: the existing query string and its bind shape stay
bit-identical to today, so the planner keeps using the same indexes when
fairness is off or no workspace is currently capped. A separate
`WORKER_PULL_QUERIES_FAIRNESS` adds `AND workspace_id <> ALL($2::text[])`
and is only materialized while the feature is enabled.
Hard-gated to `CLOUD_HOSTED=true` + BASE_URL host == app.windmill.dev at
three layers: frontend `cloudonly: true`, API setter rejection in
`set_global_setting_internal`, runtime check in `fairness_active`. Settings
are exposed under Jobs in the instance-settings UI; defaults are off so
the change is a no-op for self-hosted.
Two-pass pull guarantees no worker idling: if every queued job belongs to
a capped workspace, the second pass uses the unmodified pull queries.
Cap re-asserts on the next refresh.
Fixes WIN-1982
* fix(queue): address CI review findings on workspace fairness
Six fixes from the four-reviewer cross-check on #9303:
1. **Aggregation evaluation (Codex P1).** The previous `INSERT ... ON CONFLICT
DO UPDATE WHERE updated_at < ...` had the heavy `v2_job_queue ∪
v2_job_completed` aggregation inlined into `VALUES`, which Postgres
evaluates for every contender to build the proposed row — losing the
"one heavy aggregation per cycle cluster-wide" property the design
advertises. Split into three small statements: (a) cheap claim with
constant `VALUES`, (b) winner-only `UPDATE ... SET value = jsonb_build_object('overloaded', <agg>)`
(Postgres only evaluates `SET` per row matching `WHERE`, so losers never
compute the aggregation), (c) read for everyone. Heavy query now truly
runs ~0.2-0.5 qps cluster-wide regardless of fleet size.
2. **Numeric setting wraparound (cubic P1).** `u64 as u32` and downstream
`u32 as i32` could silently flip sign and feed `make_interval(secs => -N)`,
making `now() - interval` a future timestamp and disabling the
completed-jobs half of the activity signal. Clamp `duration_secs` to
[1, 86400] and `min_total_jobs` to [0, u32::MAX] before storing.
3. **`/instance_config` bypass (cubic/Claude/Codex P2).** Bulk config endpoint
sidestepped `set_global_setting_internal`'s gate; a self-hosted superadmin
could persist `workspace_fairness_*` rows via the bulk path. Mirror the
per-key check in `set_instance_config` upsert flow.
4. **DB error coerced to false (Claude P2).** `load_workspace_fairness_enabled`
collapsed `Err(_)` to `false` and unconditionally swapped the atomic — a
transient DB blip during notify-event propagation toggled the feature off
cluster-wide (and triggered a `store_pull_query` rebuild precisely when load
is highest). Now propagates the error so the atomic stays at its prior value.
5. **Refresh failure cooldown (Claude P2).** Storing `0` removed the rate
limit entirely; every subsequent pull spawned a new refresh task. Leave
`LAST_REFRESH_MICROS` at `now_us` (already written by the CAS) so the
natural interval acts as the cooldown.
6. **Visibility + duplication (Pi P2).** Mark `make_pull_query_fairness` as
`pub(crate)`. Move the duplicated `BASE_URL host == app.windmill.dev`
parser into `windmill-common::worker::is_cloud_production_host` and share
it between the API setter and the runtime path.
Verified locally:
- `POST /api/settings/global/workspace_fairness_enabled` → 400 (per-key gate)
- `PUT /api/settings/instance_config` with fairness key → 400 (bulk gate)
- `cargo check --workspace --features=private,enterprise,quickjs` — clean
Refs WIN-1982.
* fix(queue): second round of CI review nits on workspace fairness
Three issues raised by the Codex/Claude re-review of commit 0b38ff2:
1. Non-cloud deletes were rejected (Codex P2). The cloud gate ran before
the Null / empty-string deletion branches in both `set_global_setting_internal`
and the bulk `set_instance_config`. A self-hosted instance that inherited
stale `workspace_fairness_*` rows from a cloned cloud DB couldn't clear
them through the API — the rows stayed in `global_settings` and continued
to show up in the YAML export. Now the gate only blocks upserts; Null /
empty-string deletes pass through on any host.
2. Deleted numeric knobs kept stale runtime values (Codex P2). When a
cloud admin cleared `workspace_fairness_max_percent`, `..._duration_secs`,
or `..._min_total_jobs`, the notify-event fired but the numeric loaders
ignored `Ok(None)` and left the previous in-memory value pinned until
process restart. Loaders now distinguish three outcomes:
- `Err(_)`: transient — leave atomic alone (preserves the
previous-round fix).
- `Ok(None)` / `Ok(Some(invalid))`: reset to the documented default.
- `Ok(Some(valid))`: clamp and store.
Defaults are extracted to `WORKSPACE_FAIRNESS_*_DEFAULT` constants kept
in sync with the `AtomicU32::new(...)` initialisers in
`windmill-common/src/worker.rs`.
3. `fairness_active` was `pub` with no cross-crate caller (Claude nit).
Tightened to module-private.
Verified locally on this non-cloud instance:
POST .../workspace_fairness_enabled body=null → 200 (delete passes)
POST .../workspace_fairness_enabled body=true → 400 (set blocked)
PUT .../instance_config {} → 200 (no-op passes)
PUT .../instance_config with fairness key → 400 (bulk set blocked)
Skipped the partial index on `v2_job_queue WHERE running = true` that
Claude flagged as a residual nit — queue stays under 50k rows per the
operator's measurement, so the seq-scan cost (~10 ms × 0.5 qps =
~0.5% of a DB core) is well below the noise floor and the index isn't
worth the maintenance cost on job transitions.
Refs WIN-1982.
This commit is contained in:
+26
-2
@@ -59,7 +59,9 @@ use windmill_common::{
|
||||
RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING, SAML_METADATA_SETTING,
|
||||
SCIM_TOKEN_SETTING, SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING, TEAMS_SETTING,
|
||||
TIMEOUT_WAIT_RESULT_SETTING, UV_EXCLUDE_NEWER_SETTING, UV_INDEX_STRATEGY_SETTING,
|
||||
UV_PYTHON_INSTALL_MIRROR_SETTING, WORKSPACE_REGISTRIES_SETTING,
|
||||
UV_PYTHON_INSTALL_MIRROR_SETTING, WORKSPACE_FAIRNESS_DURATION_SECS_SETTING,
|
||||
WORKSPACE_FAIRNESS_ENABLED_SETTING, WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING,
|
||||
WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, WORKSPACE_REGISTRIES_SETTING,
|
||||
},
|
||||
scripts::ScriptLang,
|
||||
stats_oss::schedule_stats,
|
||||
@@ -120,7 +122,9 @@ use crate::monitor::{
|
||||
initial_load, load_disable_password_login, load_fork_workspace_tag_append_fork_suffix,
|
||||
load_keep_job_dir, load_metrics_debug_enabled, load_preview_tags_override,
|
||||
load_require_preexisting_user, load_tag_per_workspace_enabled,
|
||||
load_tag_per_workspace_workspaces, monitor_db, reload_app_workspaced_route_setting,
|
||||
load_tag_per_workspace_workspaces, load_workspace_fairness_duration_secs,
|
||||
load_workspace_fairness_enabled, load_workspace_fairness_max_percent,
|
||||
load_workspace_fairness_min_total, monitor_db, reload_app_workspaced_route_setting,
|
||||
reload_audit_log_retention_days_setting, reload_base_url_setting,
|
||||
reload_bun_install_min_release_age_setting, reload_bunfig_install_scopes_setting,
|
||||
reload_critical_alert_mute_ui_setting, reload_critical_alerts_on_token_expiry_setting,
|
||||
@@ -1765,6 +1769,26 @@ async fn process_notify_event(
|
||||
tracing::error!("Error loading preview tags override: {e:#}");
|
||||
}
|
||||
}
|
||||
WORKSPACE_FAIRNESS_ENABLED_SETTING => {
|
||||
if let Err(e) = load_workspace_fairness_enabled(db).await {
|
||||
tracing::error!("Error loading workspace fairness enabled: {e:#}");
|
||||
}
|
||||
}
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING => {
|
||||
if let Err(e) = load_workspace_fairness_max_percent(db).await {
|
||||
tracing::error!("Error loading workspace fairness max percent: {e:#}");
|
||||
}
|
||||
}
|
||||
WORKSPACE_FAIRNESS_DURATION_SECS_SETTING => {
|
||||
if let Err(e) = load_workspace_fairness_duration_secs(db).await {
|
||||
tracing::error!("Error loading workspace fairness duration secs: {e:#}");
|
||||
}
|
||||
}
|
||||
WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING => {
|
||||
if let Err(e) = load_workspace_fairness_min_total(db).await {
|
||||
tracing::error!("Error loading workspace fairness min total: {e:#}");
|
||||
}
|
||||
}
|
||||
SMTP_SETTING => {
|
||||
reload_smtp_config(db).await;
|
||||
}
|
||||
|
||||
+126
-1
@@ -69,6 +69,8 @@ use windmill_common::{
|
||||
RETENTION_PERIOD_SECS_SETTING, SAML_METADATA_SETTING, SCIM_TOKEN_SETTING,
|
||||
STORE_AUDIT_LOGS_S3_SETTING, TIMEOUT_WAIT_RESULT_SETTING, UV_EXCLUDE_NEWER_SETTING,
|
||||
UV_INDEX_STRATEGY_SETTING, UV_PYTHON_INSTALL_MIRROR_SETTING,
|
||||
WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, WORKSPACE_FAIRNESS_ENABLED_SETTING,
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING,
|
||||
},
|
||||
indexer::load_indexer_config,
|
||||
jwt::JWT_SECRET,
|
||||
@@ -84,7 +86,8 @@ use windmill_common::{
|
||||
store_suspended_pull_query, Connection, WorkerConfig, DEFAULT_TAGS_PER_WORKSPACE,
|
||||
DEFAULT_TAGS_WORKSPACES, FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX, INDEXER_CONFIG,
|
||||
PREVIEW_TAGS_OVERRIDE, SCRIPT_TOKEN_EXPIRY, SMTP_CONFIG, WINDMILL_DIR, WORKER_CONFIG,
|
||||
WORKER_GROUP,
|
||||
WORKER_GROUP, WORKSPACE_FAIRNESS_DURATION_SECS, WORKSPACE_FAIRNESS_ENABLED,
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT, WORKSPACE_FAIRNESS_MIN_TOTAL,
|
||||
},
|
||||
KillpillSender, AUDIT_LOG_RETENTION_DAYS, BASE_URL, CRITICAL_ALERTS_ON_DB_OVERSIZE,
|
||||
CRITICAL_ALERTS_ON_TOKEN_EXPIRY, CRITICAL_ALERT_MUTE_UI_ENABLED, CRITICAL_ERROR_CHANNELS, DB,
|
||||
@@ -248,6 +251,22 @@ pub async fn initial_load(
|
||||
if let Err(e) = load_preview_tags_override(db).await {
|
||||
tracing::error!("Error loading preview tags override: {e:#}");
|
||||
}
|
||||
|
||||
// Workspace fairness (cloud-only). Load the percentage/duration/min knobs
|
||||
// *before* the enabled flag so that `load_workspace_fairness_enabled` reads
|
||||
// current values when re-storing the pull queries.
|
||||
if let Err(e) = load_workspace_fairness_max_percent(db).await {
|
||||
tracing::error!("Error loading workspace fairness max percent: {e:#}");
|
||||
}
|
||||
if let Err(e) = load_workspace_fairness_duration_secs(db).await {
|
||||
tracing::error!("Error loading workspace fairness duration secs: {e:#}");
|
||||
}
|
||||
if let Err(e) = load_workspace_fairness_min_total(db).await {
|
||||
tracing::error!("Error loading workspace fairness min total: {e:#}");
|
||||
}
|
||||
if let Err(e) = load_workspace_fairness_enabled(db).await {
|
||||
tracing::error!("Error loading workspace fairness enabled: {e:#}");
|
||||
}
|
||||
}
|
||||
|
||||
if server_mode {
|
||||
@@ -543,6 +562,112 @@ pub async fn load_preview_tags_override(db: &DB) -> error::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Upper bound on the duration window. Postgres `make_interval(secs => $1::int4)` is the consumer
|
||||
// downstream, so this stays comfortably below `i32::MAX` and the subsequent `u32 -> i32` cast in
|
||||
// `workspace_fairness::refresh_overloaded` cannot wrap into a negative interval (which would
|
||||
// silently turn `now() - interval` into a future timestamp and disable the completed-jobs half
|
||||
// of the activity signal). A day is the practical ceiling for a "rolling window" knob.
|
||||
const WORKSPACE_FAIRNESS_DURATION_SECS_MAX: u64 = 86_400;
|
||||
|
||||
/// Min-total floor is a counting threshold; cap at `u32::MAX` to make wraparound impossible
|
||||
/// while still leaving more headroom than any realistic cluster will need.
|
||||
const WORKSPACE_FAIRNESS_MIN_TOTAL_MAX: u64 = u32::MAX as u64;
|
||||
|
||||
// Defaults used when a fairness knob is unset (row missing or row deleted via NULL/empty value).
|
||||
// Must stay in sync with the `AtomicU32::new(...)` initialisers in `windmill-common/src/worker.rs`
|
||||
// so a process that has never seen the setting reads the same value as one that just saw it
|
||||
// cleared.
|
||||
const WORKSPACE_FAIRNESS_MAX_PERCENT_DEFAULT: u32 = 50;
|
||||
const WORKSPACE_FAIRNESS_DURATION_SECS_DEFAULT: u32 = 10;
|
||||
const WORKSPACE_FAIRNESS_MIN_TOTAL_DEFAULT: u32 = 4;
|
||||
|
||||
pub async fn load_workspace_fairness_enabled(db: &DB) -> error::Result<()> {
|
||||
// Match the convention used by `load_preview_tags_override` /
|
||||
// `load_fork_workspace_tag_append_fork_suffix`: on transient DB errors, leave the in-memory
|
||||
// atomic untouched rather than silently toggling the feature off across the whole cluster
|
||||
// (which would also trigger an unnecessary `store_pull_query` rebuild — exactly when DB load
|
||||
// is probably highest).
|
||||
let new_enabled =
|
||||
match load_value_from_global_settings(db, WORKSPACE_FAIRNESS_ENABLED_SETTING).await? {
|
||||
Some(serde_json::Value::Bool(t)) => t,
|
||||
// Setting unset / non-bool → explicit off.
|
||||
_ => false,
|
||||
};
|
||||
let prev = WORKSPACE_FAIRNESS_ENABLED.swap(new_enabled, Ordering::Relaxed);
|
||||
// Re-store the pull queries so the fairness variants appear/disappear in
|
||||
// lockstep with the toggle.
|
||||
if prev != new_enabled {
|
||||
let wc = windmill_common::worker::WORKER_CONFIG.load_full();
|
||||
store_pull_query(&wc).await;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn load_workspace_fairness_max_percent(db: &DB) -> error::Result<()> {
|
||||
// Distinguish three outcomes:
|
||||
// - `Err(_)`: transient DB issue. Leave the atomic alone (don't clobber a known-good value
|
||||
// because of a network blip during a notify-event propagation).
|
||||
// - `Ok(None)` or `Ok(Some(invalid))`: setting is unset / explicitly cleared / corrupt.
|
||||
// Restore the default so a deletion via the admin UI actually takes effect at runtime
|
||||
// instead of leaving the stale in-memory value pinned until restart.
|
||||
// - `Ok(Some(valid))`: clamp and store.
|
||||
match load_value_from_global_settings(db, WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING).await? {
|
||||
Some(serde_json::Value::Number(n)) => {
|
||||
let v = n
|
||||
.as_u64()
|
||||
.map(|u| u.clamp(1, 100) as u32)
|
||||
.unwrap_or(WORKSPACE_FAIRNESS_MAX_PERCENT_DEFAULT);
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT.store(v, Ordering::Relaxed);
|
||||
}
|
||||
_ => {
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT
|
||||
.store(WORKSPACE_FAIRNESS_MAX_PERCENT_DEFAULT, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn load_workspace_fairness_duration_secs(db: &DB) -> error::Result<()> {
|
||||
// See `load_workspace_fairness_max_percent` for the Err / None / invalid policy.
|
||||
match load_value_from_global_settings(db, WORKSPACE_FAIRNESS_DURATION_SECS_SETTING).await? {
|
||||
Some(serde_json::Value::Number(n)) => {
|
||||
// Clamp to the safe range before narrowing. The downstream `u32 -> i32` cast in
|
||||
// `workspace_fairness::refresh_overloaded` makes any value above `i32::MAX` toxic
|
||||
// (sign flip → negative interval → silent disable of the completed-jobs scan).
|
||||
let v = n
|
||||
.as_u64()
|
||||
.map(|u| u.clamp(1, WORKSPACE_FAIRNESS_DURATION_SECS_MAX) as u32)
|
||||
.unwrap_or(WORKSPACE_FAIRNESS_DURATION_SECS_DEFAULT);
|
||||
WORKSPACE_FAIRNESS_DURATION_SECS.store(v, Ordering::Relaxed);
|
||||
}
|
||||
_ => {
|
||||
WORKSPACE_FAIRNESS_DURATION_SECS
|
||||
.store(WORKSPACE_FAIRNESS_DURATION_SECS_DEFAULT, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn load_workspace_fairness_min_total(db: &DB) -> error::Result<()> {
|
||||
// See `load_workspace_fairness_max_percent` for the Err / None / invalid policy.
|
||||
match load_value_from_global_settings(db, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING).await? {
|
||||
Some(serde_json::Value::Number(n)) => {
|
||||
// Clamp before narrowing — same reasoning as `_duration_secs`, just for the
|
||||
// counting threshold rather than the interval.
|
||||
let v = n
|
||||
.as_u64()
|
||||
.map(|u| u.min(WORKSPACE_FAIRNESS_MIN_TOTAL_MAX) as u32)
|
||||
.unwrap_or(WORKSPACE_FAIRNESS_MIN_TOTAL_DEFAULT);
|
||||
WORKSPACE_FAIRNESS_MIN_TOTAL.store(v, Ordering::Relaxed);
|
||||
}
|
||||
_ => {
|
||||
WORKSPACE_FAIRNESS_MIN_TOTAL
|
||||
.store(WORKSPACE_FAIRNESS_MIN_TOTAL_DEFAULT, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn load_fork_workspace_tag_append_fork_suffix(db: &DB) -> error::Result<()> {
|
||||
let value =
|
||||
load_value_from_global_settings(db, FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING).await;
|
||||
|
||||
Reference in New Issue
Block a user