You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
node: emit a sandbox OOM kill metric that separates node memory pressure from the sandbox's own limit - #1576
Sandbox memory kills can now be told apart in metrics. At the moment the node only sees that a VM's process or cgroup is gone, so a sandbox that reached its own memory limit looks the same as one killed because the node ran out of memory.
When a sandbox cgroup is removed, the node reads its hierarchical memory.events. If the kernel OOM killer ended any process in it, the node counts those kills once in node_memory.sandbox_oom_kills, tagged with cgroup (vm, sidecar) and cause:
sandbox_limit: the cgroup, or one inside it, reached its own memory.max.
node_pressure: the sandbox stayed within its own limit, so the kill came from above it (the sandbox parent's cap or a host-wide OOM).
The node also logs a sandbox cgroup had OOM kills warning with the sandbox id, cause, kill count and limit hits. The counting happens in remove_cgroup_path, so both normal teardown and orphan reclaim report. It only reports after the directory is actually removed, so a retried removal never counts the same kills twice. docs/observability.md documents the metric.
Validation
cargo test -p pplx-space-node-runtime-core --features host-runtime --lib cgroup:: (21 passed). This includes new tests for parsing kernel memory.events, classifying the cause, and the counter's tags for vm and sidecar cgroups.
cargo clippy with -D warnings on the changed crates.
Not validated on a real cgroup v2 host with an actual OOM kill. The fixtures use the kernel's memory.events format.
Severity and risk
Severity: medium. Without this, the cause of a memory kill can only be found in the kernel log.
Risk: low. This only adds observability. A missing or unreadable events file skips reporting with a debug log and does not change removal. One known limit: a sandbox that ever hit its own limit is counted as sandbox_limit, even if a later kill came from node pressure. To roll back, revert this change.
Not covered
OOM kills inside the guest happen in the guest kernel and are not counted here.
AI assistance: Perplexity Computer drafted the code, tests, docs and this description.
Why: OOM reporting adds a read to sandbox teardown. Confirm its effect on removal in the supported host environment before merging.
Commit: 2ac0de8d9335.
No actionable comments were generated in the recent review.
Walkthrough
The runtime reads memory.events before removing a sandbox cgroup. After successful removal, it reports positive OOM-kill counts with cgroup and cause tags. The cause is classified as sandbox_limit or node_pressure. The change also adds tests and observability documentation.
Possibly related PRs
perplexityai/SPACE#1541: Removes pplx-space-metrics, which this change uses to define and report the OOM metric.
perplexityai/SPACE#1515: Removes the test-capture feature enabled here for testing OOM metric tags.
Merge Risk: Moderate · up to 2ac0d
OOM reporting adds a read to sandbox teardown. Confirm its effect on removal in the supported host environment before merging.
Security architecture risk: Low · up to 2ac0d
The change adds bounded observability without changing sandbox memory limits or cleanup permissions. No new security exposure was identified. Remaining uncertainty concerns real-host reporting behavior and the accuracy of cause attribution, rather than a demonstrated isolation failure.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
observed — The reporting hook covers normal sandbox termination and node-local orphan reclaim through their shared removal helper. It reports host-side VM and sidecar cgroup events; OOM kills within the guest kernel are outside this metric.
Trust Boundaries and Controls
observed — Normal cleanup derives paths from a typed SandboxId and checks quiescence before removal. Orphan discovery accepts directory names only when their stripped identity parses as SandboxId, excludes tracked sandboxes, and checks population before reclamation. The PR leaves these selection controls unchanged.
Resilience and Maintainability Implications
inferred — Telemetry failure does not veto cleanup, and failed or interrupted removals do not emit a report. Successful-removal gating avoids duplicate reports from ordinary retries, but it is not a durable exactly-once guarantee: process interruption after directory removal and before emission can lose the report.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title identifies the node area and summarizes the sandbox OOM kill metric. It meets the specified format requirements.
Description Follows The Pr Template
The PR description contains all four required items: Summary, Validation with a test result, Severity and risk, and an AI assistance line that names the tool and its contributions.
Bug Fixes Name A Regression Test
The PR adds OOM-kill observability. The diff does not show a fix for an application behavior defect, so the check does not apply.
Weakened Safeguards Are Justified
The diff does not remove or weaken a test, assertion, lint, CI check, or repository guard. It adds tests and replaces an empty successful-removal branch with metric reporting.
Public-Safe Content
The diff, title, and description contain no content covered by the failure criteria. The paths in the diff are generic test paths.
Recent review info
Automatic approval policy
Commit: 2ac0de8d9335.
Setting
Effective value
Source
Automatic approvals
Enabled
Global configuration
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
Count OOM kills in each sandbox cgroup once, when it is removed, tagged by
whether the sandbox reached its own memory limit or the kill came from node
memory pressure.
shehan-pplx
changed the title
node: report sandbox OOM kills by cause
node: emit a sandbox OOM kill metric that separates node memory pressure from the sandbox's own limit
Nov 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sandbox memory kills can now be told apart in metrics. At the moment the node only sees that a VM's process or cgroup is gone, so a sandbox that reached its own memory limit looks the same as one killed because the node ran out of memory.
When a sandbox cgroup is removed, the node reads its hierarchical
memory.events. If the kernel OOM killer ended any process in it, the node counts those kills once innode_memory.sandbox_oom_kills, tagged withcgroup(vm,sidecar) andcause:sandbox_limit: the cgroup, or one inside it, reached its ownmemory.max.node_pressure: the sandbox stayed within its own limit, so the kill came from above it (the sandbox parent's cap or a host-wide OOM).The node also logs a
sandbox cgroup had OOM killswarning with the sandbox id, cause, kill count and limit hits. The counting happens inremove_cgroup_path, so both normal teardown and orphan reclaim report. It only reports after the directory is actually removed, so a retried removal never counts the same kills twice.docs/observability.mddocuments the metric.Validation
cargo test -p pplx-space-node-runtime-core --features host-runtime --lib cgroup::(21 passed). This includes new tests for parsing kernelmemory.events, classifying the cause, and the counter's tags forvmandsidecarcgroups.cargo clippywith-D warningson the changed crates.Not validated on a real cgroup v2 host with an actual OOM kill. The fixtures use the kernel's
memory.eventsformat.Severity and risk
Severity: medium. Without this, the cause of a memory kill can only be found in the kernel log.
Risk: low. This only adds observability. A missing or unreadable events file skips reporting with a debug log and does not change removal. One known limit: a sandbox that ever hit its own limit is counted as
sandbox_limit, even if a later kill came from node pressure. To roll back, revert this change.Not covered
OOM kills inside the guest happen in the guest kernel and are not counted here.
AI assistance: Perplexity Computer drafted the code, tests, docs and this description.