mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix(alerts): identify server replica in low-disk alert + per-host dedup tag (#10143)
* fix(alerts): identify server replica in low-disk alert + per-host dedup tag The server-mode low-disk alert keyed its dedup tag on the mountpoint alone, so `simple_alert_helper` mapped every server replica onto a single alert row per mountpoint. With more than one replica that row flaps every monitor pass: a replica seeing low disk raises the alert while a replica seeing healthy disk recovers it. The alert text also could not say which replica tripped. The fix lives in windmill-ee-private (`low_disk_alerts` in windmill-common/src/ee.rs) and appends the hostname to both the message and the dedup tag, mirroring the worker branch. Also add a regression test pinning the server tag as per-host, and correct the monitor cadence comments: iterations are LISTEN_NEW_EVENTS_INTERVAL_SEC (10s by default), not 30s, so "~60s (2 iterations * 30s)" was wrong on both factors. * fix(alerts): widen healthchecks.check_type so per-host disk tags fit Alert tags embed a mountpoint and a hostname, both unbounded, but check_type was varchar(50). create_alert only logs the insert error while the notification still fires, so an overflowing tag re-alerts every monitor pass and never records recovery state. The server tag overflows for ordinary pod-length hostnames, and the existing worker tag already overflows for every tracked mount except "/". Widening the column fixes both; bounding the hostname would not, since the mountpoint alone can consume the budget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to b3d01f2c0d2c0714ae95b8a348af22b0fcc30ee4 This commit updates the EE repository reference after PR #666 was merged in windmill-ee-private. Previous ee-repo-ref: ccd1e42cf6b2d051ca17074fbdf5b80a46cffe0f New ee-repo-ref: b3d01f2c0d2c0714ae95b8a348af22b0fcc30ee4 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
+1
-1
@@ -5,7 +5,7 @@
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Varchar"
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
|
||||
@@ -1 +1 @@
|
||||
d8a7ac6ae97642a7f4928e6be6846a32dabf4e26
|
||||
b3d01f2c0d2c0714ae95b8a348af22b0fcc30ee4
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
-- Rows written since the up migration may exceed 50 chars; truncate so the cast succeeds.
|
||||
ALTER TABLE healthchecks ALTER COLUMN check_type TYPE varchar(50) USING left(check_type, 50);
|
||||
@@ -0,0 +1,4 @@
|
||||
-- Alert tags embed unbounded components (mountpoint, hostname), so they overflowed
|
||||
-- varchar(50). create_alert only logs the insert error while the notification still
|
||||
-- fires, so an overflowing tag re-alerts every monitor pass and never recovers.
|
||||
ALTER TABLE healthchecks ALTER COLUMN check_type TYPE text;
|
||||
+10
-8
@@ -3255,9 +3255,10 @@ pub async fn monitor_db(
|
||||
}
|
||||
};
|
||||
|
||||
// run every ~60s (2 iterations * 30s). Enterprise feature: core logic is
|
||||
// in `crate::ee` (OSS gets a no-op stub); gated on a valid Enterprise
|
||||
// license, mirroring how `audit_log()` itself is license-aware.
|
||||
// run every 2 iterations (~20s at the default LISTEN_NEW_EVENTS_INTERVAL_SEC).
|
||||
// Enterprise feature: core logic is in `crate::ee` (OSS gets a no-op stub);
|
||||
// gated on a valid Enterprise license, mirroring how `audit_log()` itself
|
||||
// is license-aware.
|
||||
let export_audit_logs_to_object_store_f = async {
|
||||
#[cfg(feature = "parquet")]
|
||||
if server_mode && iteration.is_some() && iteration.as_ref().unwrap().should_run(2) {
|
||||
@@ -3281,11 +3282,12 @@ pub async fn monitor_db(
|
||||
}
|
||||
};
|
||||
|
||||
// run every ~60s (2 iterations * 30s). Enterprise feature: the active
|
||||
// `// freshness` backstop lives in windmill-queue's `freshness_watchdog`
|
||||
// (`private`); OSS gets a no-op stub. Runtime-gated on an Enterprise
|
||||
// license like the audit export above. Safe on concurrent servers — the
|
||||
// watchdog claims per-script state rows atomically before pushing.
|
||||
// run every 2 iterations (~20s at the default LISTEN_NEW_EVENTS_INTERVAL_SEC).
|
||||
// Enterprise feature: the active `// freshness` backstop lives in
|
||||
// windmill-queue's `freshness_watchdog` (`private`); OSS gets a no-op stub.
|
||||
// Runtime-gated on an Enterprise license like the audit export above. Safe
|
||||
// on concurrent servers — the watchdog claims per-script state rows
|
||||
// atomically before pushing.
|
||||
let pipeline_freshness_watchdog_f = async {
|
||||
if server_mode
|
||||
&& !*DISABLE_FRESHNESS_WATCHDOG
|
||||
|
||||
@@ -99,7 +99,7 @@ group_: workspace_id(char), name(char), summary(text), extra_perms(jsonb)
|
||||
FK: (workspace_id) -> workspace(id)
|
||||
group_permission_history: id(bigint), workspace_id(char), group_name(char), changed_by(char), changed_at(ts), change_type(char), member_affected(char)
|
||||
FK: (workspace_id, group_name) -> group_(workspace_id, name)
|
||||
healthchecks: id(bigint), check_type(char), healthy(bool), created_at(ts)
|
||||
healthchecks: id(bigint), check_type(text), healthy(bool), created_at(ts)
|
||||
http_trigger: path(char), route_path(char), route_path_key(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), authentication_method(authentication_method), http_method(http_method), static_asset_config(jsonb), is_static_website(bool), workspaced_route(bool), wrap_body(bool), raw_string(bool), authentication_resource_path(char), summary(char), description(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), request_type(request_type), mode(trigger_mode), labels(text[])
|
||||
input: id(uuid), workspace_id(char), runnable_id(char), runnable_type(runnable_type), name(text), args(jsonb), created_at(ts), created_by(char), is_public(bool)
|
||||
FK: (workspace_id) -> workspace(id)
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
//! Regression test for the server-mode low-disk alert dedup tag.
|
||||
//!
|
||||
//! ## Requirements
|
||||
//!
|
||||
//! - PostgreSQL database running locally
|
||||
//! - Enterprise features enabled
|
||||
//!
|
||||
//! ## Running the tests
|
||||
//!
|
||||
//! ```bash
|
||||
//! cargo test -p windmill-common --test low_disk_alerts --features private,enterprise -- --ignored --nocapture
|
||||
//! ```
|
||||
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
mod tests {
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::ee::low_disk_alerts;
|
||||
use windmill_common::utils::HOSTNAME;
|
||||
|
||||
/// The server tag must carry the hostname: `simple_alert_helper` keys one alert row per
|
||||
/// tag, so a host-less tag lets a replica seeing low disk and a replica seeing free disk
|
||||
/// raise and recover the same row every monitor pass.
|
||||
///
|
||||
/// The hostname is forced to a pod-length name so the tag runs past 50 chars, which
|
||||
/// `check_type` must stay wide enough to hold: `create_alert` only logs the insert
|
||||
/// error while the notification still fires, so a tag that does not fit re-alerts every
|
||||
/// pass and never recovers. Asserting the row persists pins the width and the shape.
|
||||
#[ignore = "requires database setup - run with --ignored flag"]
|
||||
#[sqlx::test(migrations = "../migrations")]
|
||||
async fn server_low_disk_tag_is_per_host(db: Pool<Postgres>) {
|
||||
// Both statics are lazy and read on first access inside the call below.
|
||||
std::env::set_var("FORCE_HOSTNAME", "windmill-server-7d9f8b6c4d-x2k9p");
|
||||
// Force every mount to read as low so the server branch raises.
|
||||
std::env::set_var("MIN_FREE_DISK_SPACE_MB", "999999999999");
|
||||
|
||||
low_disk_alerts(&db, true, false, vec![]).await;
|
||||
|
||||
let tags: Vec<String> = sqlx::query_scalar(
|
||||
"SELECT check_type FROM healthchecks WHERE check_type LIKE 'low-disk-v2-server@%'",
|
||||
)
|
||||
.fetch_all(&db)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
!tags.is_empty(),
|
||||
"expected at least one server low-disk alert; an alert whose tag does not fit \
|
||||
check_type is dropped here while its notification still fires"
|
||||
);
|
||||
for tag in &tags {
|
||||
assert!(
|
||||
tag.ends_with(&format!("@{}", *HOSTNAME)),
|
||||
"server tag {tag} is not per-host; replicas would share one alert row"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user