starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-18 08:01:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: keep non traffic-serving processes out of coordinated restarts (#10694)

Browse Source 
* fix: key server_heartbeat row on hostname so restarts reuse one row

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: trim announce_server_started doc to the durable constraints

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: only traffic-serving processes take part in coordinated restarts

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: name every non traffic-serving mode in the restart-gate comments

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: narrow the restart-gate comments to claims that hold

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-08-14 15:10:30 +02:00
committed by GitHub
parent 878b8ef4c4
commit 6d03784d4b
2 changed files with 65 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/main.rs
+41 -10
View File
@@ -1691,7 +1691,8 @@ async fn process_notify_event(
"restart_worker_group" => {
if worker_mode && payload == *WORKER_GROUP {
tracing::info!("Restart requested for worker group '{payload}'");
spawn_graceful_killpill(tx, db, 30, "worker group restart requested").await;
spawn_graceful_killpill(tx, db, 30, "worker group restart requested", server_mode)
.await;
}
}
"notify_webhook_change" => {
@@ -1997,8 +1998,14 @@ async fn process_notify_event(
reload_otel_tracing_proxy_setting(conn).await;
if worker_mode {
tracing::info!("OTEL tracing proxy setting changed, restarting worker");
spawn_graceful_killpill(tx, db, 30, "OTEL tracing proxy setting change")
.await;
spawn_graceful_killpill(
tx,
db,
30,
"OTEL tracing proxy setting change",
server_mode,
)
.await;
}
}
REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING => {
@@ -2009,12 +2016,20 @@ async fn process_notify_event(
}
EXPOSE_METRICS_SETTING => {
tracing::info!("Metrics setting changed, restarting");
spawn_graceful_killpill(tx, db, 30, "metrics setting change").await;
spawn_graceful_killpill(tx, db, 30, "metrics setting change", server_mode)
.await;
}
EMAIL_DOMAIN_SETTING => {
tracing::info!("Email domain setting changed");
if server_mode {
spawn_graceful_killpill(tx, db, 30, "email domain setting change").await;
spawn_graceful_killpill(
tx,
db,
30,
"email domain setting change",
server_mode,
)
.await;
}
}
EXPOSE_DEBUG_METRICS_SETTING => {
@@ -2050,19 +2065,26 @@ async fn process_notify_event(
}
OTEL_SETTING => {
tracing::info!("OTEL setting changed, restarting");
spawn_graceful_killpill(tx, db, 30, "OTEL setting change").await;
spawn_graceful_killpill(tx, db, 30, "OTEL setting change", server_mode).await;
}
REQUEST_SIZE_LIMIT_SETTING => {
if server_mode {
tracing::info!("Request limit size change detected, killing server expecting to be restarted");
spawn_graceful_killpill(tx, db, 30, "request size limit change").await;
spawn_graceful_killpill(
tx,
db,
30,
"request size limit change",
server_mode,
)
.await;
}
}
SAML_METADATA_SETTING => {
tracing::info!(
"SAML metadata change detected, killing server expecting to be restarted"
);
spawn_graceful_killpill(tx, db, 30, "SAML metadata change").await;
spawn_graceful_killpill(tx, db, 30, "SAML metadata change", server_mode).await;
}
HUB_BASE_URL_SETTING => {
if let Err(e) = reload_hub_base_url_setting(conn, server_mode).await {
@@ -2279,16 +2301,24 @@ pub async fn run_workers(
/// then the sleep+kill is spawned in the background so the notification handler is not blocked.
///
/// Falls back to drain-only delay if DB coordination fails.
///
/// Only `server_mode` processes coordinate, on the strength of the worker case: a worker
/// group restarting costs queue latency rather than lost work, `v2_job_queue` being durable.
/// Were workers to take part, one could claim the `is_first` slot and leave every server
/// holding its shutdown open for a peer that serves no API traffic.
async fn spawn_graceful_killpill(
tx: &KillpillSender,
db: &Pool<Postgres>,
safety_margin_secs: u64,
context: &str,
server_mode: bool,
) {
// Minimum delay before any restart to let in-flight requests drain
const DRAIN_DELAY_SECS: u64 = 3;
let (delay, is_first) =
let (delay, is_first) = if !server_mode {
(DRAIN_DELAY_SECS, true)
} else {
match coordinate_restart_delay(db, safety_margin_secs, DRAIN_DELAY_SECS).await {
Ok(r) => r,
Err(e) => {
@@ -2298,7 +2328,8 @@ async fn spawn_graceful_killpill(
);
(DRAIN_DELAY_SECS, true)
}
};
}
};
tracing::info!(
"Scheduling {context} graceful shutdown in {delay}s (first_to_restart={is_first})"
backend/windmill-api/src/lib.rs
+24 -12
View File
@@ -1166,8 +1166,10 @@ pub async fn run_server(
}
// Announce this server is ready so coordinated restarts can detect a healthy peer.
if let Err(e) = announce_server_started(&db).await {
tracing::warn!("Failed to announce server started: {e:#}");
if server_mode {
if let Err(e) = announce_server_started(&db).await {
tracing::warn!("Failed to announce server started: {e:#}");
}
}
let server = server.with_graceful_shutdown(async move {
@@ -1314,25 +1316,35 @@ pub async fn wait_for_db_migrations(
const SERVER_HEARTBEAT_TASK: &str = "server_heartbeat";
/// Write a server-started heartbeat to `background_task_state` so that
/// other instances waiting to restart can detect this server is healthy.
/// Write a server-started heartbeat to `background_task_state` so that other
/// traffic-serving instances waiting to restart can detect this one is healthy.
///
/// Only `server_mode` processes announce, since only they are peers worth waiting for:
/// `spawn_graceful_killpill` holds a shutdown open to keep the API answered, and a worker,
/// indexer or MCP process coming up is no evidence that it is.
///
/// The row is keyed per host and `owner` per process, and both halves carry weight.
/// `INSTANCE_NAME` is random per start, so a row keyed on it never conflicts and
/// accumulates one row per start; `owner` is what tells a peer's start from its own
/// when processes share a host.
async fn announce_server_started(db: &DB) -> anyhow::Result<()> {
use windmill_common::INSTANCE_NAME;
use windmill_common::{utils::HOSTNAME, INSTANCE_NAME};
let instance = INSTANCE_NAME.as_str();
let host = HOSTNAME.as_str();
sqlx::query(
"INSERT INTO background_task_state (name, value, running, owner, started_at, updated_at)
VALUES ($1, '\"started\"'::jsonb, true, $2, NOW(), NOW())
ON CONFLICT (name)
DO UPDATE SET updated_at = NOW(), running = true, owner = $2",
DO UPDATE SET started_at = NOW(), updated_at = NOW(), running = true, owner = $2",
)
.bind(format!("{SERVER_HEARTBEAT_TASK}:{instance}"))
.bind(format!("{SERVER_HEARTBEAT_TASK}:{host}"))
.bind(instance)
.execute(db)
.await?;
tracing::info!("Announced server started for instance {instance}");
tracing::info!("Announced server started for instance {instance} on host {host}");
Ok(())
}
@@ -1362,10 +1374,10 @@ pub async fn check_any_server_started(db: &DB, not_before: chrono::DateTime<chro
}
/// Delete `server_heartbeat:*` rows that have not been refreshed in a long
/// time. Each server startup generates a fresh random `INSTANCE_NAME` and
/// inserts a new row keyed by `server_heartbeat:{instance}`; because that
/// row is only written once (on startup) and never updated thereafter, the
/// table grows by one row per server restart and is otherwise never pruned.
/// time. A restart in place reuses its host's row (see
/// `announce_server_started`), but a host that never comes back leaves one
/// behind, and hosts are disposable wherever the hostname carries a generated
/// pod or container id.
///
/// The row is only consulted by `check_any_server_started`, which itself
/// filters on `updated_at > not_before` (the moment a restart was initiated),