Skip to content

fix(ipc): rate-limit and contextualize high-memory warnings - #5962

Merged
Bobronium merged 6 commits into
mainfrom
arseny/contextualize-memory-warnings
Jun 4, 2026
Merged

fix(ipc): rate-limit and contextualize high-memory warnings#5962
Bobronium merged 6 commits into
mainfrom
arseny/contextualize-memory-warnings

Conversation

@Bobronium

Copy link
Copy Markdown
Member

Changes

  • Rate-limit the warning: at most once per _MEMORY_WARN_COOLDOWN (120s) per process, but re-emits early if usage grew by ≥_MEMORY_WARN_RESET_DELTA_MB (50MB) since the last warning, so real leaks still surface promptly.
  • Reword the message and mark it advisory when no hard limit is configured.
  • Add extra: uptime, has_running_job, baseline_memory_mb, growth_memory_mb (current − baseline).
  • process_kind property: base = "process", overridden to "job process" / "inference process".

Behavior

  • No change to when a process is killed — the memory_limit_mb path is untouched except for richer log context. Warning thresholds and defaults unchanged.
  • Log message string and extra keys change

Testing

  • New unit tests (tests/test_supervised_proc_memory.py): cooldown suppression, growth-based re-emit, baseline/growth reporting, default process_kind.

Bobronium and others added 4 commits June 4, 2026 00:29
The "process memory usage is high" warning was emitted on every memory
sample (~every 5s), so a process sitting above the warn threshold spammed
the logs. The message also looked alarming and lacked the context needed
to tell a process that started heavy from one whose usage grew over time.

- Rate-limit the warning with a cooldown, re-emitting early only if usage
  grew significantly since the last warning (so real leaks still surface).
- Reword the message and mark it advisory when no hard limit is configured
  (no process is terminated in that case).
- Add diagnostic context: process uptime, whether a job is running, the
  post-prewarm baseline RSS, and growth since that baseline.
- Add unit tests for the cooldown, growth re-emit, and logging context.

Co-Authored-By: Claude <noreply@anthropic.com>
…nic clock

The memory warning hardcoded "job process", but SupervisedProc is also the
base for the inference process — it can't know which it is. Introduce an
overridable `process_kind` property (base: "process", job executor: "job
process", inference executor: "inference process") and use it in the
memory log messages.

Also switch process uptime and the warning cooldown from time.time() to
time.monotonic(), so a wall-clock adjustment can't skew them.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@chenghao-mou
chenghao-mou requested a review from a team June 4, 2026 00:40

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@longcw longcw left a comment

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.

lgtm!

@theomonnom theomonnom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

Comment thread livekit-agents/livekit/agents/ipc/job_proc_executor.py Outdated
Replace the optional string `process_kind` with an abstract property returning
a `SupervisedProcKind` enum (`job`, `inference`). Subclassing `SupervisedProc`
now requires declaring a kind, which keeps the memory-warning log from ever
mislabelling a process (the previous string default was a footgun).

The enum overrides `__str__` so f-string interpolation renders as the bare
value ("job process …", "inference process …") on Python 3.10 — `StrEnum` is
3.11+ and the project still targets 3.10.

Test fakes (`_DummySupervisedProc`, `_FakeProc`) now declare a kind. Added a
test that verifies the enum stringifies cleanly and one that verifies a
subclass missing `process_kind` raises TypeError at instantiation.

Co-Authored-By: Claude <noreply@anthropic.com>
@Bobronium
Bobronium requested a review from theomonnom June 4, 2026 18:44
Comment thread livekit-agents/livekit/agents/ipc/supervised_proc.py Outdated
Co-Authored-By: Claude <noreply@anthropic.com>
@Bobronium
Bobronium merged commit 1c4bfa4 into main Jun 4, 2026
23 checks passed
@Bobronium
Bobronium deleted the arseny/contextualize-memory-warnings branch June 4, 2026 19:35
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.

3 participants