Skip to content

fix(oom): prevent unbounded memory growth causing heap OOM crashes - #2965

Merged
diegosouzapw merged 3 commits into
cheaper-inference:mainfrom
soyelmismo:fix/oom-memory-leak-caches
May 31, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:mainfrom
soyelmismo:fix/oom-memory-leak-caches

Conversation

@soyelmismo

@soyelmismo soyelmismo commented May 31, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The Node.js process crashes with FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory within ~5 minutes of intensive use.

Root causes identified during investigation:

  1. comboMetrics.ts — metrics and shadowMetrics Maps grow unboundedly (no TTL, no eviction, no cleanup timer)
  2. usage.ts — 6 subscription/model caches with passive-only TTL (entries never proactively purged)
  3. providerRegistry.ts — all 212 provider entries materialized eagerly at module load, even if unused

Changes

1. open-sse/services/comboMetrics.ts — Cache eviction + TTL

  • MAX_METRICS_ENTRIES = 500 with LRU eviction via lastUsedAt
  • METRICS_TTL_MS = 1 hour
  • Cleanup setInterval every 5 minutes (.unref()'d)
  • Size-cap checks in recordComboRequest, recordComboShadowRequest, recordComboIntent

2. open-sse/services/usage.ts — Proactive TTL purging

  • SUB_CACHE_TTL_MS = 10 minutes
  • Cleanup setInterval every 5 minutes (.unref()'d)
  • Purges 4 data caches; 2 inflight Maps left alone (self-clean on Promise resolution)

3. open-sse/config/providerRegistry.ts — Lazy Proxy

  • REGISTRY wrapped in a Proxy — individual provider entries materialized on first access only
  • _byAlias, _unsupportedParamsMap, _passthroughProviderIds all deferred until first lookup
  • getRegisteredProviders() returns pre-computed key list without eager iteration

4. scripts/check-permissions.sh — Entrypoint integration

  • Entrypoint reads OMNIROUTE_MEMORY_MB and builds NODE_OPTIONS dynamically
  • This was documented in .env.example but never actually implemented

Verification

  • TypeScript compilation: zero new errors (pre-existing mcp-server/server.ts errors confirmed on main)
  • All existing public API signatures preserved
  • No function logic changes — only additive eviction/TTL/lazy-loading code
  • .unref() on all timers so they don't prevent clean process exit

soyelmismo added 2 commits May 30, 2026 20:59
…stry

Root cause: Node.js process crashes with OOM (JavaScript heap out of memory)
within 5 minutes of intensive use. Heap exhausted at ~250MB.

Three fixes targeting the memory leak sources identified during investigation:

1. comboMetrics.ts — Add eviction + TTL for metrics/shadowMetrics Maps
   - MAX_METRICS_ENTRIES = 500 (LRU eviction via lastUsedAt)
   - METRICS_TTL_MS = 1 hour
   - Cleanup interval every 5 minutes (unref'd)
   - Size-cap checks in recordComboRequest, recordComboShadowRequest, recordComboIntent

2. usage.ts — Proactive TTL purging for 6 passive subscription caches
   - SUB_CACHE_TTL_MS = 10 minutes
   - Cleanup interval every 5 minutes (unref'd)
   - Purges: geminiCliSubCache, antigravitySubCache, antigravityAvailableModelsCache,
     antigravityCreditProbeCache
   - Inflight Maps left alone (self-clean on Promise resolution)

3. providerRegistry.ts — Lazy Proxy for 212 provider entries
   - REGISTRY renamed to _REGISTRY_EAGER, wrapped in lazy Proxy
   - Individual entries materialized on first access only (_registryCache Map)
   - _byAlias, _unsupportedParamsMap, _passthroughProviderIds all made lazy
   - getRegisteredProviders() returns pre-computed _registryKeys (no eager iteration)

Also noted: Dockerfile hardcodes --max-old-space-size=256 (too low for production).
.env.example documents OMNIROUTE_MEMORY_MB but no script reads it — entrypoint
should be updated separately to respect this setting.

TypeScript: zero new errors introduced (pre-existing mcp-server/server.ts errors
confirmed on main branch)
…verride

Dockerfile: OMNIROUTE_MEMORY_MB=1024 (was hardcoded 256MB).
NODE_OPTIONS now references OMNIROUTE_MEMORY_MB so users can
override via docker run -e OMNIROUTE_MEMORY_MB=2048.

check-permissions.sh: entrypoint reads OMNIROUTE_MEMORY_MB and
builds NODE_OPTIONS dynamically. This was documented in .env.example
but never actually implemented — the entrypoint just did exec
without processing the variable.

Combined with the first commit's cache eviction fixes, this should
prevent the OOM crashes that killed the process within 5 minutes
of intensive use.
@soyelmismo
soyelmismo requested a review from diegosouzapw as a code owner May 31, 2026 02:53

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces memory management improvements, including configurable Node memory limits via OMNIROUTE_MEMORY_MB, a lazy registry proxy for provider configurations, and proactive TTL cache eviction mechanisms in comboMetrics and usage services. Feedback highlights several critical issues: the shell script incorrectly overrides the runtime memory limit due to flag ordering; the metrics eviction and cleanup logic fails to cover shadowMetrics and risks premature eviction; the lazy registry proxy adds complexity without actual memory savings since the registry is eagerly traversed at startup; and the lack of new tests for the modified production code violates Repository Style Guide Rule 9.

Comment thread scripts/check-permissions.sh Resolved
Comment thread open-sse/services/comboMetrics.ts Resolved
Comment thread open-sse/services/comboMetrics.ts Resolved
Comment thread open-sse/services/comboMetrics.ts Resolved
Comment thread open-sse/services/comboMetrics.ts Resolved
Comment thread open-sse/services/comboMetrics.ts Resolved
Comment thread open-sse/config/providerRegistry.ts Resolved
Comment thread open-sse/services/usage.ts Resolved
@soyelmismo
soyelmismo marked this pull request as draft May 31, 2026 02:56
@kilo-code-bot

kilo-code-bot Bot commented May 31, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
open-sse/config/providerRegistry.ts 4264 getOwnPropertyDescriptor trap returns value from _REGISTRY_EAGER directly, not from cache. When get trap caches a value, getOwnPropertyDescriptor still returns the original reference, causing stale descriptors and potential memory duplication. Should check cache first.
File Line Issue
open-sse/services/usage.ts 1839 Proactive TTL cleanup uses SUB_CACHE_TTL_MS (10 minutes) for all 4 caches, but individual caches have different intended TTLs (ANTIGRAVITY_CACHE_TTL_MS=5min, ANTIGRAVITY_MODELS_CACHE_TTL_MS=1min, ANTIGRAVITY_CREDIT_PROBE_TTL_MS=5min). This causes antigravityAvailableModelsCache to persist ~10x longer than intended (10min vs 1min).
Other Observations (not in diff)

No issues found in unchanged code.

Files Reviewed (4 files)
  • Dockerfile - No issues
  • open-sse/config/providerRegistry.ts - 1 issue
  • open-sse/services/comboMetrics.ts - No issues
  • open-sse/services/usage.ts - 1 issue
  • scripts/check-permissions.sh - No issues

Reviewed by nemotron-3-super-120b-a12b-20230311:free · 62,614 tokens

Comment thread open-sse/config/providerRegistry.ts Resolved
Comment thread open-sse/services/usage.ts Resolved
1. check-permissions.sh: swap NODE_OPTIONS flag order so runtime
   OMNIROUTE_MEMORY_MB override wins (last flag takes precedence)

2. comboMetrics.ts: evictOldestMetric(targetMap) now accepts the
   target map as parameter; cleanup timer iterates BOTH metrics
   AND shadowMetrics; null lastUsedAt falls back to Date.now()
   instead of epoch 0 (prevents premature eviction of intent-only
   entries)

3. providerRegistry.ts: remove Proxy wrapper — adds CPU/complexity
   overhead with zero memory savings since _REGISTRY_EAGER is
   already fully allocated and generator functions iterate all
   entries at startup. Simple re-export instead.

4. usage.ts: use per-cache TTL constants in cleanup timer
   (ANTIGRAVITY_MODELS_CACHE_TTL_MS=1min, others=5min) instead of
   single 10min SUB_CACHE_TTL_MS for all caches.

5. Add 18 unit tests for comboMetrics memory management covering:
   basic CRUD, shadow metrics, intent tracking, eviction at capacity,
   strategy tracking, and reset operations.
@soyelmismo
soyelmismo marked this pull request as ready for review May 31, 2026 03:34
@diegosouzapw
diegosouzapw merged commit 009ad13 into cheaper-inference:main May 31, 2026
3 checks passed
diegosouzapw added a commit that referenced this pull request May 31, 2026
fix(oom): prevent unbounded memory growth causing heap OOM crashes
@diegosouzapw diegosouzapw mentioned this pull request Jun 2, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
1. check-permissions.sh: swap NODE_OPTIONS flag order so runtime
   OMNIROUTE_MEMORY_MB override wins (last flag takes precedence)

2. comboMetrics.ts: evictOldestMetric(targetMap) now accepts the
   target map as parameter; cleanup timer iterates BOTH metrics
   AND shadowMetrics; null lastUsedAt falls back to Date.now()
   instead of epoch 0 (prevents premature eviction of intent-only
   entries)

3. providerRegistry.ts: remove Proxy wrapper — adds CPU/complexity
   overhead with zero memory savings since _REGISTRY_EAGER is
   already fully allocated and generator functions iterate all
   entries at startup. Simple re-export instead.

4. usage.ts: use per-cache TTL constants in cleanup timer
   (ANTIGRAVITY_MODELS_CACHE_TTL_MS=1min, others=5min) instead of
   single 10min SUB_CACHE_TTL_MS for all caches.

5. Add 18 unit tests for comboMetrics memory management covering:
   basic CRUD, shadow metrics, intent tracking, eviction at capacity,
   strategy tracking, and reset operations.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
1. check-permissions.sh: swap NODE_OPTIONS flag order so runtime
   OMNIROUTE_MEMORY_MB override wins (last flag takes precedence)

2. comboMetrics.ts: evictOldestMetric(targetMap) now accepts the
   target map as parameter; cleanup timer iterates BOTH metrics
   AND shadowMetrics; null lastUsedAt falls back to Date.now()
   instead of epoch 0 (prevents premature eviction of intent-only
   entries)

3. providerRegistry.ts: remove Proxy wrapper — adds CPU/complexity
   overhead with zero memory savings since _REGISTRY_EAGER is
   already fully allocated and generator functions iterate all
   entries at startup. Simple re-export instead.

4. usage.ts: use per-cache TTL constants in cleanup timer
   (ANTIGRAVITY_MODELS_CACHE_TTL_MS=1min, others=5min) instead of
   single 10min SUB_CACHE_TTL_MS for all caches.

5. Add 18 unit tests for comboMetrics memory management covering:
   basic CRUD, shadow metrics, intent tracking, eviction at capacity,
   strategy tracking, and reset operations.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
1. check-permissions.sh: swap NODE_OPTIONS flag order so runtime
   OMNIROUTE_MEMORY_MB override wins (last flag takes precedence)

2. comboMetrics.ts: evictOldestMetric(targetMap) now accepts the
   target map as parameter; cleanup timer iterates BOTH metrics
   AND shadowMetrics; null lastUsedAt falls back to Date.now()
   instead of epoch 0 (prevents premature eviction of intent-only
   entries)

3. providerRegistry.ts: remove Proxy wrapper — adds CPU/complexity
   overhead with zero memory savings since _REGISTRY_EAGER is
   already fully allocated and generator functions iterate all
   entries at startup. Simple re-export instead.

4. usage.ts: use per-cache TTL constants in cleanup timer
   (ANTIGRAVITY_MODELS_CACHE_TTL_MS=1min, others=5min) instead of
   single 10min SUB_CACHE_TTL_MS for all caches.

5. Add 18 unit tests for comboMetrics memory management covering:
   basic CRUD, shadow metrics, intent tracking, eviction at capacity,
   strategy tracking, and reset operations.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ory-leak-caches

fix(oom): prevent unbounded memory growth causing heap OOM crashes
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.

2 participants