Skip to content

benchmarking: add OOM kills to server telemetry - #2391

Merged
Max Smythe (maxsmythe) merged 1 commit into
agent-substrate:mainfrom
Nishanth29:oom-events
Oct 17, 2026
Merged

Max Smythe (maxsmythe) merged 1 commit into
agent-substrate:mainfrom
Nishanth29:oom-events

Conversation

@Nishanth29

@Nishanth29 Nishanth Kotla (Nishanth29) commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1590

Adds an oom_events block to server telemetry:

  • node, pod: OOM kills over the run, from cAdvisor's container_oom_events_total. node counts global OOMs. pod counts kills inside a worker pod (actor sandboxes and the worker container) and is best effort: a kill whose cgroup is gone before the scrape is missed.
  • worker_restarts, worker_oomkilled_pods, evicted, worker_pods_lost: from the Kubernetes API, read before and after the run. evicted comes from Evicted events, as atecontroller deletes an evicted pod at once.

Other changes:

  • monitoring.yaml keeps container_oom_events_total.
  • The runner's benchmark-workloads Role can also list events.
  • The README explains what each field does and doesn't catch.

Live check (gVisor, 3m):

  • Actor limit 256Mi, 2 users: pod = 6, matching 6 kernel oom-kill lines, one per actor sandbox. Other fields 0; the worker stayed up.

  • No actor limit, 8 users: the kernel killed the whole worker twice at the node-allocatable limit (oom_memcg=/kubepods.slice), and the kubelet evicted it. evicted = 7 (1 eviction plus 6 replacement pods the kubelet refused to start), worker_pods_lost = 1, node = pod = 0, as documented.

  • 80 measurements in the jsonl row.

  • Tests pass

  • Appropriate changes to documentation are included in the PR

@Nishanth29

Copy link
Copy Markdown
Contributor Author

@Nishanth29

Copy link
Copy Markdown
Contributor Author

Da Huang (@git286) Tim Bai (@baizhenyu), adding you as reviewers.

What this uses:

  • Node-level OOMs and worker pod restarts/evictions: cAdvisor and the k8s API (outside Substrate).
  • Kills inside the worker pod (pod, mostly actor sandboxes): cAdvisor too, since there's no Substrate OOM metric to read yet.
  • Not caught: OOMs handled inside the sandbox (a microVM guest, or a single container hitting its own limit under gVisor).

If you'd rather add an actor OOM-kill counter in Substrate (e.g. from the oom_kill read in #2258), pod can switch to that or I can drop pod from this PR until then.

@git286

Copy link
Copy Markdown
Collaborator

Let's keep pod on cAdvisor for now. #2258 only logs oom_kill in a WARN, so there's no Substrate metric to read yet. I'll file a follow-up for an ate.sandbox.oom_kills counter under #2212; once it lands, pod can switch to it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in #2386 (comment). Let's address the naming and get #2386 merged first. LGTM otherwise.

@Nishanth29

Copy link
Copy Markdown
Contributor Author

Haowei Cai (Roy) (@roycaihw) Rebased onto the updated #2386 branch and updated ateom -> worker here too (WORKER_CONTAINER, OOM_POD, tests, and README). Ready once #2386 merges!

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

auto-merge was automatically disabled October 17, 2026 02:13

Head branch was pushed to by a user without write access

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxsmythe
Max Smythe (maxsmythe) added this pull request to the merge queue Oct 17, 2026
Merged via the queue into agent-substrate:main with commit 343ca92 Oct 17, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants