diff --git a/backend/src/cgroups.rs b/backend/src/cgroups.rs index c6120c3524..285b28dfe7 100644 --- a/backend/src/cgroups.rs +++ b/backend/src/cgroups.rs @@ -42,24 +42,73 @@ pub fn disable_oom_group() -> Result<(), CgroupError> { let oom_group_file = cgroup_path.join("memory.oom.group"); if !oom_group_file.exists() { + tracing::warn!( + "memory.oom.group not found at {:?} — cgroups v2 memory controller may not be enabled. \ + OOM killer may kill the entire pod instead of individual jobs", + oom_group_file + ); return Err(CgroupError::NotSupported); } let current = fs::read_to_string(&oom_group_file)?; if current.trim() == "0" { - tracing::info!("memory.oom.group already disabled"); + tracing::info!("memory.oom.group already disabled at {:?}", cgroup_path); return Ok(()); } + tracing::info!( + "memory.oom.group is currently '{}' at {:?}, attempting to disable", + current.trim(), + cgroup_path + ); + match fs::write(&oom_group_file, "0") { Ok(_) => { - tracing::info!("Disabled memory.oom.group at {:?}", cgroup_path); + // Verify the write took effect + match fs::read_to_string(&oom_group_file) { + Ok(val) if val.trim() == "0" => { + tracing::info!("Disabled memory.oom.group at {:?}", cgroup_path); + } + Ok(val) => { + tracing::error!( + "Wrote 0 to memory.oom.group but read back '{}' at {:?}. \ + OOM killer may kill the entire pod instead of individual jobs", + val.trim(), + cgroup_path + ); + return Err(CgroupError::Io(std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "memory.oom.group write did not take effect, read back '{}'", + val.trim() + ), + ))); + } + Err(e) => { + tracing::warn!( + "Wrote 0 to memory.oom.group but could not verify at {:?}: {e}", + cgroup_path + ); + } + } Ok(()) } Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { - tracing::error!("Failed to disable memory.oom.group (need privileged mode)"); + tracing::error!( + "Failed to disable memory.oom.group at {:?} (permission denied). \ + The container needs SYS_RESOURCE capability or privileged mode. \ + OOM killer WILL kill the entire pod instead of individual jobs", + oom_group_file + ); Err(CgroupError::PermissionDenied) } - Err(e) => Err(CgroupError::Io(e)), + Err(e) => { + tracing::error!( + "Failed to disable memory.oom.group at {:?}: {e}. \ + OOM killer may kill the entire pod instead of individual jobs", + oom_group_file + ); + Err(CgroupError::Io(e)) + } } } diff --git a/backend/src/main.rs b/backend/src/main.rs index 08d59c9dd9..aa9f16af89 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -870,7 +870,55 @@ async fn windmill_main() -> anyhow::Result<()> { if worker_mode { #[cfg(any(target_os = "linux"))] if let Err(e) = disable_oom_group() { - tracing::warn!("failed to disable oom group: {:?}", e); + tracing::warn!( + "Failed to disable cgroup OOM group kill: {e:?}. \ + When a job exceeds memory, the OOM killer will kill the entire pod \ + instead of just the offending job process" + ); + } + + // Lower the worker's oom_score_adj so the OOM killer strongly prefers killing + // job subprocesses (oom_score_adj=1000) over the worker itself. + // Kubernetes sets it high for burstable QoS (e.g. 937), leaving a tiny gap vs jobs. + // Requires CAP_SYS_RESOURCE to lower it; if missing, we just warn. + #[cfg(any(target_os = "linux"))] + match std::fs::read_to_string("/proc/self/oom_score_adj") { + Ok(current) => { + let current = current.trim().to_string(); + let current_val = match current.parse::() { + Ok(v) => v, + Err(e) => { + tracing::warn!("Could not parse oom_score_adj '{current}': {e}"); + 0 + } + }; + if current_val > 0 { + match std::fs::write("/proc/self/oom_score_adj", "0") { + Ok(_) => { + tracing::info!( + "Lowered worker oom_score_adj from {current} to 0 \ + (jobs get 1000, gap=1000)" + ); + } + Err(e) => { + tracing::warn!( + "Could not lower worker oom_score_adj from {current} to 0: {e}. \ + Gap to jobs is only {} — OOM killer may target the worker instead. \ + Add CAP_SYS_RESOURCE to the container to fix this", + 1000 - current_val + ); + } + } + } else { + tracing::info!( + "Worker oom_score_adj={current} (jobs get 1000, gap={})", + 1000 - current_val + ); + } + } + Err(e) => { + tracing::warn!("Could not read worker oom_score_adj: {e}"); + } } } diff --git a/backend/windmill-worker/src/handle_child.rs b/backend/windmill-worker/src/handle_child.rs index 205da6700a..06db11ab24 100644 --- a/backend/windmill-worker/src/handle_child.rs +++ b/backend/windmill-worker/src/handle_child.rs @@ -128,17 +128,22 @@ pub async fn handle_child( let pid = child.id(); #[cfg(target_os = "linux")] if let Some(pid) = pid { - //set the highest oom priority - if let Some(mut file) = File::create(format!("/proc/{pid}/oom_score_adj")) - .await - .map_err(|e| { - tracing::error!("Could not create oom_score_file to pid {pid}: {e:#}"); - e - }) - .ok() - { - let _ = file.write_all(b"1000").await; - let _ = file.sync_all().await; + //set the highest oom priority so OOM killer targets this job, not the worker + match File::create(format!("/proc/{pid}/oom_score_adj")).await { + Ok(mut file) => { + if let Err(e) = file.write_all(b"1000").await { + tracing::error!("Failed to write oom_score_adj for pid {pid}: {e:#}"); + } + if let Err(e) = file.sync_all().await { + tracing::warn!("Failed to sync oom_score_adj for pid {pid}: {e:#}"); + } + } + Err(e) => { + tracing::error!( + "Could not open /proc/{pid}/oom_score_adj: {e:#}. \ + OOM killer may target the worker instead of this job" + ); + } } } else { tracing::info!("could not get child pid");