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

fix: improve OOM killer observability for debugging pod-level kills (#8398)

Browse Source 
* fix: improve OOM killer observability for debugging pod-level kills

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

* fix: lower worker oom_score_adj to protect it from OOM killer

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

* fix: address PR review feedback on OOM observability

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-03-17 00:49:21 +00:00
committed by GitHub
parent de5b13b840
commit fd41cd12b4
3 changed files with 118 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/cgroups.rs
+53 -4
View File
@@ -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))
}
}
}
backend/src/main.rs
+49 -1
View File
@@ -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::<i32>() {
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}");
}
}
}
backend/windmill-worker/src/handle_child.rs
+16 -11
View File
@@ -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");