mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
perf: resolve the worker external IP in the background (#10697)
* perf: resolve the worker external IP in the background `run_workers` awaited `external_ip::get_ip()` — an HTTPS GET to hub.windmill.dev — before spawning any worker, so every worker process paid that round trip before its first job pull. Measured on a CE debug build it was 120-450 ms of a ~200-500 ms startup, and behind a firewall the call does not fail fast: it burns its whole 5 s connect timeout, on every process start. That cost is per-job under EXIT_AFTER_N_JOBS. The value is informational (it is only written to `worker_ping.ip`, which the workers list displays so users can whitelist the address), so nothing needs to wait on it. It now resolves into a process-wide cache off the startup path, and `WORKER_EXTERNAL_IP` supplies it explicitly for deployments that know their egress address or have no egress at all. Until it resolves the ping carries no IP, which `insert_ping_query` now COALESCEs so a reclaimed row keeps the address the previous process wrote instead of being blanked. The main loop reports the IP as soon as it lands rather than on the next periodic tick, so a short-lived process still records it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep unknown worker IPs out of the whitelist alert Review follow-ups: - `WhitelistIp` filtered only the `'unretrievable IP'` sentinel, so the `'NO IP'` one a pending or failed lookup now leaves in the row would be offered as an address to whitelist. It filters both. - Register `WORKER_EXTERNAL_IP` in `ENV_SETTINGS` so operators can confirm from the instance settings view that it took effect. - The worker tracked whether it had reported the IP by re-reading the cache after each ping rather than remembering what the ping carried, so a lookup landing mid-ping marked it reported without it reaching the row. The value is read once and threaded through `insert_ping` / `update_worker_ping_full`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: report a sentinel IP once the lookup has definitively failed Keeping the previous process's address on a reclaimed `worker_ping` row is right while the lookup is still in flight, but not once it has failed: the row would advertise an address nothing has confirmed, and the whitelist alert would offer it. A failed lookup now reports `UNKNOWN_IP`, leaving NULL to mean "in flight". `WORKER_EXTERNAL_IP` is rejected when longer than the `varchar(50)` column rather than panicking the worker on its initial ping, which is a hard failure. Adds the regression guard for the `ON CONFLICT` semantics: reverting to `ip = EXCLUDED.ip` would compile and blank every reclaimed row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep the agent initial ping acceptable to older servers An agent worker routinely runs against a server of a different version, and one predating the background lookup rejects an initial ping carrying no IP — which `run_worker` turns into a panic, so a newly upgraded agent would crash-loop against it. The not-resolved-yet case goes over the wire as the sentinel instead, and the server maps it back so a reclaimed row still keeps its address while resolution is pending. Also documents `ip` as the one conditional exception to `insert_ping_query`'s "only `started_at` and `jobs_executed` survive a restart", and adds `WORKER_EXTERNAL_IP` to the README env-var table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: deliver the resolved IP to servers that only take it at registration A server predating the background lookup applies `ip` from the initial ping only, and ignores it on the periodic ones. An agent registering before its lookup resolves would therefore keep the sentinel forever on such a server, where it used to report its real address. It registers a second time once the address is known, skipping that when the address is still unknown, when the server is reached over SQL and needs no second registration, or once a job has run, since registering clears the row's current job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: re-register the resolved IP even after a job has run Gating the second registration on "this process has not run a job yet" meant an agent that pulled queued work before its lookup resolved never delivered the address to a server that only takes one at registration. No job of the worker is in flight where that runs, so the gate bought nothing beyond the last job's id, which the next job refills. Documents the two cases where WORKER_EXTERNAL_IP stops being an optimisation and becomes the only way to report an address: an agent against such a server, and a process shorter-lived than the lookup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert: drop the WORKER_EXTERNAL_IP escape hatch Supplying the address by hand skips the hub lookup, which is not something to make easy. Resolving it in the background is what keeps it off the startup path; opting out of it is a separate decision this does not need to take. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: distinguish an IP never established from one that could not be retrieved `NO IP` was doing double duty: the column default for a row whose lookup has not resolved, and the marker for one that failed. An operator reading the workers list could not tell "not resolved yet" from "this instance cannot reach the hub", and the latter is the actionable one. A failed lookup now reports `unretrievable IP`, which is also what it reported before the lookup moved off the startup path. That leaves `NO IP` meaning only "no address established", which is what an agent sends while its lookup is in flight and what the server maps back to "unresolved" — so the wire sentinel no longer collides with the failure marker, and an agent delivers the failure to a server that only reads an IP at registration. 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:
+4
-3
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2,\n occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus),\n memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12 WHERE worker = $6",
|
||||
"query": "UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2,\n occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus),\n memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12, ip = COALESCE($13, ip) WHERE worker = $6",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
@@ -16,10 +16,11 @@
|
||||
"Float4",
|
||||
"Float4",
|
||||
"Float4",
|
||||
"Bool"
|
||||
"Bool",
|
||||
"Varchar"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "a41c4cbaffdb714e4a963557de5a4011744d684eb24e03cb4beae6a512613159"
|
||||
"hash": "0c18351237816fe0c56e23801fcb8e70dbffcf08ed121e55c871f727c4ddf626"
|
||||
}
|
||||
-33
@@ -1,33 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)\n DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = EXCLUDED.ip, custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL\n RETURNING jobs_executed",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "jobs_executed",
|
||||
"type_info": "Int4"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Varchar",
|
||||
"Varchar",
|
||||
"Varchar",
|
||||
"TextArray",
|
||||
"Varchar",
|
||||
"Varchar",
|
||||
"TextArray",
|
||||
"Varchar",
|
||||
"Int8",
|
||||
"Int8",
|
||||
"Text",
|
||||
"Bool"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "9d9fbcb598c582a29d65be87a1e35baa410f93be3cac4b8dfd33ddbef446d3fe"
|
||||
}
|
||||
+33
@@ -0,0 +1,33 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, COALESCE($3, 'NO IP'), $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)\n DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = COALESCE($3, worker_ping.ip), custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL\n RETURNING jobs_executed",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "jobs_executed",
|
||||
"type_info": "Int4"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Varchar",
|
||||
"Varchar",
|
||||
"Text",
|
||||
"TextArray",
|
||||
"Varchar",
|
||||
"Varchar",
|
||||
"TextArray",
|
||||
"Varchar",
|
||||
"Int8",
|
||||
"Int8",
|
||||
"Text",
|
||||
"Bool"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "c748617e060bc41b5922df4b433f20b6971b993e0671b6ccb45db5ef028550bc"
|
||||
}
|
||||
+1
-8
@@ -2183,12 +2183,7 @@ pub async fn run_workers(
|
||||
// #[cfg(tokio_unstable)]
|
||||
// let monitor = tokio_metrics::TaskMonitor::new();
|
||||
|
||||
let ip = windmill_common::external_ip::get_ip()
|
||||
.await
|
||||
.unwrap_or_else(|e| {
|
||||
tracing::warn!(error = e.to_string(), "failed to get external IP");
|
||||
"unretrievable IP".to_string()
|
||||
});
|
||||
windmill_common::external_ip::resolve_ip_in_background();
|
||||
|
||||
let mut handles = Vec::with_capacity(num_workers as usize);
|
||||
|
||||
@@ -2232,7 +2227,6 @@ pub async fn run_workers(
|
||||
let conn1 = wk_conf.conn.clone();
|
||||
let worker_name = wk_conf.worker_name.clone();
|
||||
WORKERS_NAMES.write().await.push(worker_name.clone());
|
||||
let ip = ip.clone();
|
||||
let rx = killpill_rxs.pop().unwrap();
|
||||
let tx = tx.clone();
|
||||
let base_internal_url = base_internal_url.clone();
|
||||
@@ -2249,7 +2243,6 @@ pub async fn run_workers(
|
||||
worker_name,
|
||||
i as u64,
|
||||
num_workers as u32,
|
||||
&ip,
|
||||
rx,
|
||||
tx,
|
||||
&base_internal_url,
|
||||
|
||||
@@ -241,7 +241,6 @@ fn spawn_workers(
|
||||
worker_name,
|
||||
i as u64,
|
||||
n as u32,
|
||||
"127.0.0.1",
|
||||
rx,
|
||||
tx2,
|
||||
&base_internal_url,
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::{external_ip::UNKNOWN_IP, worker::insert_ping_query};
|
||||
|
||||
async fn insert_ping(db: &Pool<Postgres>, worker: &str, ip: Option<&str>) -> anyhow::Result<()> {
|
||||
insert_ping_query(
|
||||
"test-instance",
|
||||
worker,
|
||||
"default",
|
||||
ip,
|
||||
&[],
|
||||
None,
|
||||
None,
|
||||
"test",
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
false,
|
||||
db,
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The external IP resolves in the background, so the initial ping often has none yet. That must
|
||||
/// not blank the address a previous process wrote to the row this one reclaims — worker names are
|
||||
/// stable across restarts under EXIT_AFTER_N_JOBS.
|
||||
#[sqlx::test]
|
||||
async fn unresolved_ip_keeps_the_reclaimed_rows_address(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
insert_ping(&db, "wk-reclaimed", Some("1.2.3.4")).await?;
|
||||
insert_ping(&db, "wk-reclaimed", None).await?;
|
||||
let ip: String = sqlx::query_scalar("SELECT ip FROM worker_ping WHERE worker = $1")
|
||||
.bind("wk-reclaimed")
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(ip, "1.2.3.4");
|
||||
|
||||
insert_ping(&db, "wk-fresh", None).await?;
|
||||
let ip: String = sqlx::query_scalar("SELECT ip FROM worker_ping WHERE worker = $1")
|
||||
.bind("wk-fresh")
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(ip, UNKNOWN_IP);
|
||||
Ok(())
|
||||
}
|
||||
@@ -12,8 +12,58 @@
|
||||
//! connections to be from whitelisted IP addresses.
|
||||
|
||||
use crate::utils::configure_client;
|
||||
use std::sync::OnceLock;
|
||||
use std::time::Duration;
|
||||
|
||||
/// No address has ever been established for the row. Matches the `worker_ping.ip` column default,
|
||||
/// and doubles as what an agent sends while its lookup is in flight, since a server that predates
|
||||
/// the lookup being asynchronous rejects an initial ping carrying nothing.
|
||||
pub const UNKNOWN_IP: &str = "NO IP";
|
||||
|
||||
/// The lookup ran and could not produce an address. Distinct from [`UNKNOWN_IP`] because it tells
|
||||
/// an operator the difference between "never asked" and "asked, and this instance cannot reach the
|
||||
/// hub", which is the actionable one. Both are filtered out of the addresses the frontend offers
|
||||
/// for whitelisting.
|
||||
pub const UNRETRIEVABLE_IP: &str = "unretrievable IP";
|
||||
|
||||
/// `worker_ping.ip` is `VARCHAR(50)`, and a failed initial ping takes the worker down, so an
|
||||
/// overlong value must not reach the insert.
|
||||
const MAX_IP_LEN: usize = 50;
|
||||
|
||||
static EXTERNAL_IP: OnceLock<String> = OnceLock::new();
|
||||
|
||||
/// The external IP of this process, [`UNRETRIEVABLE_IP`] once the lookup has failed, or `None`
|
||||
/// while it is still in flight.
|
||||
pub fn cached_ip() -> Option<&'static str> {
|
||||
EXTERNAL_IP.get().map(String::as_str)
|
||||
}
|
||||
|
||||
/// Resolves the external IP into the process-wide cache without blocking the caller. The value is
|
||||
/// informational, and behind a firewall the lookup burns its whole 5s connect timeout on every
|
||||
/// process start, so nothing on the worker startup path may wait on it.
|
||||
pub fn resolve_ip_in_background() {
|
||||
tokio::spawn(async {
|
||||
let ip = get_ip()
|
||||
.await
|
||||
.map(|ip| {
|
||||
if ip.len() > MAX_IP_LEN {
|
||||
tracing::error!("external IP lookup returned an overlong value, ignoring it");
|
||||
UNRETRIEVABLE_IP.to_string()
|
||||
} else {
|
||||
ip
|
||||
}
|
||||
})
|
||||
.unwrap_or_else(|e| {
|
||||
tracing::warn!(
|
||||
error = e.to_string(),
|
||||
"failed to get external IP, workers of this process will report it as unretrievable"
|
||||
);
|
||||
UNRETRIEVABLE_IP.to_string()
|
||||
});
|
||||
let _ = EXTERNAL_IP.set(ip);
|
||||
});
|
||||
}
|
||||
|
||||
pub async fn get_ip() -> anyhow::Result<String> {
|
||||
tokio::select! {
|
||||
biased;
|
||||
|
||||
@@ -30,6 +30,7 @@ use crate::{
|
||||
agent_workers::PingJobStatusResponse,
|
||||
cache::{unwrap_or_error, RawNode, RawScript},
|
||||
error::{self, to_anyhow},
|
||||
external_ip::UNKNOWN_IP,
|
||||
global_settings::CUSTOM_TAGS_SETTING,
|
||||
indexer::TantivyIndexerSettings,
|
||||
server::Smtp,
|
||||
@@ -1749,25 +1750,23 @@ pub async fn update_ping_http(
|
||||
insert_ping.occupancy_rate_5m,
|
||||
insert_ping.occupancy_rate_30m,
|
||||
insert_ping.native_mode.unwrap_or(false),
|
||||
insert_ping.ip.as_deref(),
|
||||
db,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
PingType::Initial => {
|
||||
if insert_ping.worker_instance.is_none()
|
||||
|| insert_ping.version.is_none()
|
||||
|| insert_ping.ip.is_none()
|
||||
{
|
||||
return Err(anyhow::anyhow!(
|
||||
"Worker instance, version and ip are required"
|
||||
));
|
||||
if insert_ping.worker_instance.is_none() || insert_ping.version.is_none() {
|
||||
return Err(anyhow::anyhow!("Worker instance and version are required"));
|
||||
}
|
||||
|
||||
insert_ping_query(
|
||||
&insert_ping.worker_instance.unwrap(),
|
||||
&worker_name,
|
||||
worker_group,
|
||||
&insert_ping.ip.unwrap(),
|
||||
// An agent worker sends the sentinel rather than nothing, to stay acceptable to
|
||||
// servers that still require an IP here; both mean "not resolved yet".
|
||||
insert_ping.ip.as_deref().filter(|ip| *ip != UNKNOWN_IP),
|
||||
insert_ping.tags.unwrap_or_default().as_slice(),
|
||||
insert_ping.dw,
|
||||
insert_ping.dws.as_deref(),
|
||||
@@ -1904,12 +1903,13 @@ pub async fn fetch_raw_script_from_app_query(
|
||||
/// `wm_version`, hold the instance-wide `MIN_VERSION` back forever, and one still naming the
|
||||
/// job that process was killed mid-way through skews the zombie/OOM diagnostics that read it.
|
||||
/// `started_at` and `jobs_executed` are the only two columns carried over, being the
|
||||
/// continuity itself.
|
||||
/// continuity itself — plus `ip` for as long as `ip` is `None`, which means the external IP
|
||||
/// lookup has not resolved yet and the predecessor's address is still the best guess.
|
||||
pub async fn insert_ping_query(
|
||||
worker_instance: &str,
|
||||
worker_name: &str,
|
||||
worker_group: &str,
|
||||
ip: &str,
|
||||
ip: Option<&str>,
|
||||
tags: &[String],
|
||||
dw: Option<String>,
|
||||
dws: Option<&[String]>,
|
||||
@@ -1920,9 +1920,13 @@ pub async fn insert_ping_query(
|
||||
native_mode: bool,
|
||||
db: &DB,
|
||||
) -> anyhow::Result<i32> {
|
||||
// A NULL `ip` means the external IP lookup is still in flight; a later ping fills it in, and
|
||||
// meanwhile the value a previous process wrote to a reclaimed row is the best guess we have. A
|
||||
// lookup that has failed reports `external_ip::UNRETRIEVABLE_IP`, which does overwrite it. The
|
||||
// literal below must stay equal to `external_ip::UNKNOWN_IP`.
|
||||
let previous_jobs_executed = sqlx::query_scalar!(
|
||||
"INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)
|
||||
DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = EXCLUDED.ip, custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL
|
||||
"INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, COALESCE($3, 'NO IP'), $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)
|
||||
DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = COALESCE($3, worker_ping.ip), custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL
|
||||
RETURNING jobs_executed",
|
||||
worker_instance,
|
||||
worker_name,
|
||||
@@ -2027,12 +2031,13 @@ pub async fn update_worker_ping_main_loop_query(
|
||||
occupancy_rate_5m: Option<f32>,
|
||||
occupancy_rate_30m: Option<f32>,
|
||||
native_mode: bool,
|
||||
ip: Option<&str>,
|
||||
db: &DB,
|
||||
) -> anyhow::Result<()> {
|
||||
timeout(Duration::from_secs(10), sqlx::query!(
|
||||
"UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2,
|
||||
occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus),
|
||||
memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12 WHERE worker = $6",
|
||||
memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12, ip = COALESCE($13, ip) WHERE worker = $6",
|
||||
jobs_executed,
|
||||
tags,
|
||||
occupancy_rate,
|
||||
@@ -2045,6 +2050,7 @@ pub async fn update_worker_ping_main_loop_query(
|
||||
occupancy_rate_5m,
|
||||
occupancy_rate_30m,
|
||||
native_mode,
|
||||
ip,
|
||||
)
|
||||
.execute(db))
|
||||
.await??;
|
||||
|
||||
@@ -442,7 +442,6 @@ pub fn spawn_test_worker(
|
||||
let (tx, rx) = KillpillSender::new(1);
|
||||
let worker_instance: &str = "test worker instance";
|
||||
let worker_name: String = next_worker_name();
|
||||
let ip: &str = Default::default();
|
||||
let conn = conn.to_owned();
|
||||
|
||||
let tx2 = tx.clone();
|
||||
@@ -465,7 +464,6 @@ pub fn spawn_test_worker(
|
||||
worker_name,
|
||||
1,
|
||||
1,
|
||||
ip,
|
||||
rx,
|
||||
tx2,
|
||||
&base_internal_url,
|
||||
@@ -494,7 +492,6 @@ pub fn spawn_test_worker_dedicated(
|
||||
let (tx, rx) = KillpillSender::new(1);
|
||||
let worker_instance: &str = "test worker instance";
|
||||
let worker_name: String = next_worker_name();
|
||||
let ip: &str = Default::default();
|
||||
let conn = conn.to_owned();
|
||||
|
||||
let tx2 = tx.clone();
|
||||
@@ -548,7 +545,6 @@ pub fn spawn_test_worker_dedicated(
|
||||
worker_name,
|
||||
1,
|
||||
1,
|
||||
ip,
|
||||
rx,
|
||||
tx2,
|
||||
&base_internal_url,
|
||||
|
||||
@@ -41,6 +41,7 @@ use windmill_common::{
|
||||
agent_workers::DECODED_AGENT_TOKEN,
|
||||
apps::AppScriptId,
|
||||
cache::{future::FutureCachedExt, ScriptData, ScriptMetadata},
|
||||
external_ip::cached_ip,
|
||||
schema::{should_validate_schema, SchemaValidator},
|
||||
utils::{create_directory_async, WarnAfterExt},
|
||||
worker::{
|
||||
@@ -2487,7 +2488,6 @@ pub async fn run_worker(
|
||||
worker_name: String,
|
||||
i_worker: u64,
|
||||
num_workers: u32,
|
||||
ip: &str,
|
||||
mut killpill_rx: tokio::sync::broadcast::Receiver<()>,
|
||||
killpill_tx: KillpillSender,
|
||||
base_internal_url: &str,
|
||||
@@ -2583,7 +2583,8 @@ pub async fn run_worker(
|
||||
|
||||
let mut last_ping = Instant::now() - Duration::from_secs(NUM_SECS_PING + 1);
|
||||
|
||||
let previous_jobs_executed = insert_ping(hostname, &worker_name, ip, conn)
|
||||
let mut reported_ip = cached_ip();
|
||||
let previous_jobs_executed = insert_ping(hostname, &worker_name, reported_ip, conn)
|
||||
.await
|
||||
.expect("initial ping could be sent");
|
||||
|
||||
@@ -3066,7 +3067,26 @@ pub async fn run_worker(
|
||||
|
||||
otel_set_worker_uptime(&worker_name, start_time.elapsed().as_secs_f64());
|
||||
|
||||
if last_ping.elapsed().as_secs() > NUM_SECS_PING {
|
||||
// The external IP resolves in the background, after the initial ping. Pinging on the very
|
||||
// next iteration rather than the next periodic one is what gets it into the row of a worker
|
||||
// whose process is short-lived (EXIT_AFTER_N_JOBS).
|
||||
let ip = cached_ip();
|
||||
let ip_just_resolved = reported_ip.is_none() && ip.is_some();
|
||||
if ip_just_resolved || last_ping.elapsed().as_secs() > NUM_SECS_PING {
|
||||
// Servers older than the background lookup take an IP from the initial ping only, so an
|
||||
// agent has to register a second time to deliver whatever the lookup settled on, an
|
||||
// address or the unretrievable marker. Registering also clears the row's job columns,
|
||||
// which costs at most the last job's id here: no job of this worker is in flight at this
|
||||
// point in the loop, and the next one refills them.
|
||||
if ip_just_resolved && conn.as_sql().is_none() {
|
||||
if let Err(e) = insert_ping(hostname, &worker_name, ip, &conn).await {
|
||||
tracing::warn!(
|
||||
worker = %worker_name, hostname = %hostname,
|
||||
"failed to re-register with the resolved external IP: {e}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let read_cgroups =
|
||||
*REFRESH_CGROUP_READINGS && last_reading.elapsed().as_secs() > NUM_SECS_READINGS;
|
||||
update_worker_ping_full(
|
||||
@@ -3077,6 +3097,7 @@ pub async fn run_worker(
|
||||
&hostname,
|
||||
&mut occupancy_metrics,
|
||||
&killpill_tx,
|
||||
ip,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -3084,6 +3105,7 @@ pub async fn run_worker(
|
||||
last_reading = Instant::now();
|
||||
}
|
||||
last_ping = Instant::now();
|
||||
reported_ip = ip;
|
||||
}
|
||||
|
||||
if (jobs_executed as u32 + vacuum_shift) % VACUUM_PERIOD == 0 {
|
||||
|
||||
@@ -4,6 +4,7 @@ use uuid::Uuid;
|
||||
use windmill_common::{
|
||||
agent_workers::{PingJobStatus, PingJobStatusResponse},
|
||||
cache,
|
||||
external_ip::UNKNOWN_IP,
|
||||
worker::{
|
||||
get_memory, get_vcpus, get_windmill_memory_usage, get_worker_memory_usage,
|
||||
insert_ping_query, update_job_ping_query, update_worker_ping_from_job_query,
|
||||
@@ -26,6 +27,7 @@ pub(crate) async fn update_worker_ping_full(
|
||||
hostname: &str,
|
||||
occupancy_metrics: &mut OccupancyMetrics,
|
||||
killpill_tx: &KillpillSender,
|
||||
ip: Option<&str>,
|
||||
) {
|
||||
let wc = WORKER_CONFIG.load();
|
||||
let tags = wc.worker_tags.clone();
|
||||
@@ -64,6 +66,7 @@ pub(crate) async fn update_worker_ping_full(
|
||||
occupancy_rate_5m,
|
||||
occupancy_rate_30m,
|
||||
native_mode,
|
||||
ip,
|
||||
)
|
||||
})
|
||||
.retry(
|
||||
@@ -110,6 +113,7 @@ async fn update_worker_ping_full_inner(
|
||||
occupancy_rate_5m: Option<f32>,
|
||||
occupancy_rate_30m: Option<f32>,
|
||||
native_mode: bool,
|
||||
ip: Option<&str>,
|
||||
) -> anyhow::Result<()> {
|
||||
match conn {
|
||||
Connection::Sql(db) => {
|
||||
@@ -126,6 +130,7 @@ async fn update_worker_ping_full_inner(
|
||||
occupancy_rate_5m,
|
||||
occupancy_rate_30m,
|
||||
native_mode,
|
||||
ip,
|
||||
db,
|
||||
)
|
||||
.await?;
|
||||
@@ -139,7 +144,7 @@ async fn update_worker_ping_full_inner(
|
||||
last_job_executed: None,
|
||||
last_job_workspace_id: None,
|
||||
worker_instance: None,
|
||||
ip: None,
|
||||
ip: ip.map(str::to_string),
|
||||
tags: Some(tags.to_vec()),
|
||||
dw: None,
|
||||
dws: None,
|
||||
@@ -169,7 +174,7 @@ async fn update_worker_ping_full_inner(
|
||||
pub async fn insert_ping(
|
||||
worker_instance: &str,
|
||||
worker_name: &str,
|
||||
ip: &str,
|
||||
ip: Option<&str>,
|
||||
db: &Connection,
|
||||
) -> anyhow::Result<i32> {
|
||||
let (tags, dw, dws, native_mode) = {
|
||||
@@ -228,7 +233,10 @@ pub async fn insert_ping(
|
||||
last_job_executed: None,
|
||||
last_job_workspace_id: None,
|
||||
worker_instance: Some(worker_instance.to_string()),
|
||||
ip: Some(ip.to_string()),
|
||||
// Servers older than the background lookup reject an initial ping with
|
||||
// no IP, and an agent worker routinely runs against one, so the
|
||||
// not-resolved-yet case goes over the wire as the sentinel.
|
||||
ip: Some(ip.unwrap_or(UNKNOWN_IP).to_string()),
|
||||
tags: Some(tags.to_vec()),
|
||||
dw: dw,
|
||||
dws: dws,
|
||||
|
||||
@@ -4,12 +4,16 @@
|
||||
|
||||
let ips: string[] | undefined = $state(undefined)
|
||||
|
||||
// Sentinels the backend stores when a worker has no external IP to report: 'NO IP' while the
|
||||
// lookup is still in flight, 'unretrievable IP' once it has failed.
|
||||
const UNKNOWN_IPS = ['NO IP', 'unretrievable IP']
|
||||
|
||||
WorkerService.listWorkers({ pingSince: 300 }).then((workers) => {
|
||||
ips = [
|
||||
...new Set(
|
||||
workers
|
||||
.filter((worker) => {
|
||||
return worker.ip != 'unretrievable IP' && worker.last_ping && worker.last_ping < 300
|
||||
return !UNKNOWN_IPS.includes(worker.ip) && worker.last_ping && worker.last_ping < 300
|
||||
})
|
||||
.map((worker) => worker.ip)
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user