Skip to content

fix(resourcePressure): log numeric detail on every rejection, recover faster - #12293

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
hartmark:fix/resource-pressure-faster-recovery-logging
Sep 3, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
hartmark:fix/resource-pressure-faster-recovery-logging

Conversation

@hartmark

@hartmark hartmark commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Debugging a live 503 storm on omniroute-dev (currently serving real production traffic) surfaced two real gaps in the resource-pressure admission guard.

1. Rejections were nearly invisible. The admission-layer guard (services/admission/runtime.ts) rejects with a 503 before the request ever reaches chatCore/attemptLogging.ts, the only place that writes call_logs rows — so a resource-pressure rejection never shows up in /dashboard/logs, by architecture, not by bug. The only console signal was [resourcePressure] critical pressure guard tripped (reason=psi_some); returning 503 — a bare reason code with no numbers, so there was nothing to act on when deciding whether the guard is mistuned or genuinely saturated.

2. Recovery lagged more than necessary. PSI's own someAvg10/fullAvg10 are kernel-computed 10-second rolling averages, so real recovery already lags behind the actual event ending. Requiring 3 consecutive samples on top of that (at the ~1s sample cadence) stacked another 2-3 seconds of guard-still-shedding time after the process was genuinely fine again.

Why This Change Was Made

Logging: buildCriticalGuard now takes an optional detail object and logs the actual numbers at the moment of rejection — heapUsedMb/thresholdMb for the immediate heap-trip path, and psiSomeAvg10/psiFullAvg10/cgroupCurrentMb/cgroupMaxMb/recoveryStreak/sampleAgeMs for the cached-critical-state reuse path. Every rejection logs this, not just the first sample that flips state into critical.

Recovery speed: isRecovered() already requires every tracked ratio/PSI value to clear the separate, more conservative recoveryRatio/recoveryPsiAvg10 thresholds (not just dip under the critical ones) before a sample counts as recovered at all — that's real signal, not noise. The sustainedSamplesRecovery streak requirement on top of that conservative bar was redundant delay. Reduced from 3 to 1.

User Impact

An operator watching podman logs/console output during a real pressure event now sees the actual heap/PSI/cgroup numbers driving the rejection, instead of just a reason code. The guard also exits its rejecting state roughly 2-3 seconds sooner once the underlying pressure genuinely subsides, reducing user-visible failed-request duration during a real spike.

Evidence

Full resource-pressure test suite, run against this exact change:

$ node --import tsx/esm ... --test tests/unit/resource-pressure.test.ts tests/unit/resource-pressure-policy.test.ts tests/unit/resource-pressure-runtime.test.ts tests/unit/resource-pressure-sampler.test.ts
ℹ tests 36
ℹ pass 36
ℹ fail 0

$ node --import tsx/esm ... --test tests/unit/execute-chat-resource-pressure-breaker.test.ts
ℹ tests 3
ℹ pass 3
ℹ fail 0

39/39. Sample of the new log output (from the test run itself):

[resourcePressure] critical pressure guard tripped (reason=v8_heap_absolute heapUsedMb=201 thresholdMb=200); returning 503
[resourcePressure] critical pressure guard tripped (reason=v8_heap_ratio psiSomeAvg10=null psiFullAvg10=null cgroupCurrentMb=null cgroupMaxMb=null recoveryStreak=0 sampleAgeMs=11); returning 503

eslint clean on both touched files. tsc --noEmit shows 2 pre-existing errors unrelated to this change (confirmed present on the unmodified base too, in an unrelated auth/tlsFingerprint file).

Applied directly to the live omniroute-dev deployment first via hot-reload, since this was actively affecting real production traffic during debugging — confirmed the dashboard stayed healthy (200s, no compile/HMR errors) after the change. This PR is the matching clean-history change against release/v3.8.51.

… faster

Two real gaps found while debugging a live 503 storm on omniroute-dev
serving real production traffic:

1. Silent-ish rejections. The admission-layer guard (services/admission/
   runtime.ts) rejects with a 503 before the request ever reaches
   chatCore/attemptLogging.ts, the only place that writes call_logs rows
   -- so these rejections were architecturally invisible in the dashboard
   by design, and the only console signal was a bare reason code
   ("psi_some") with no numbers to act on. buildCriticalGuard now logs
   the actual heap MB / PSI avg10 / cgroup MB / recovery-streak values at
   the moment of rejection, for both the immediate heap-trip path and the
   cached-critical-state reuse path in check() -- every rejection, not
   just the first one that flips state.

2. Slow recovery. PSI's own someAvg10/fullAvg10 are kernel-computed 10s
   rolling averages, so real recovery already lags by design. Requiring
   3 consecutive samples *on top* of that (sustainedSamplesRecovery,
   ~1s cadence) stacked another 2-3s of guard-still-shedding time after
   the process was genuinely fine again. isRecovered() already requires
   every tracked ratio/PSI value to clear the separate, more conservative
   recoveryRatio/recoveryPsiAvg10 thresholds (not just dip under the
   critical ones) before a sample counts as recovered at all, so the
   streak requirement was redundant delay on an already-conservative
   bar. Reduced sustainedSamplesRecovery from 3 to 1.

Verification: full resource-pressure test suite (resource-pressure,
resource-pressure-policy, resource-pressure-runtime, resource-pressure-
sampler, execute-chat-resource-pressure-breaker) 39/39. eslint clean.
tsc --noEmit: 2 pre-existing unrelated errors confirmed present on the
unmodified base (auth/tlsFingerprint typing), none touching these files.

Applied directly to the live omniroute-dev deployment first (hot-reload,
confirmed healthy: dashboard responding, no compile/HMR errors) since
this was actively affecting real production traffic; this PR is the
matching clean-history change.
@hartmark
hartmark force-pushed the fix/resource-pressure-faster-recovery-logging branch from 4c169a0 to c041a68 Compare September 1, 2026 12:15
…e noisy-neighbor pressure

/proc/pressure/memory reflects host-wide PSI, not this process's own
cgroup pressure. On a shared host, memory contention from unrelated
processes was tripping the guard while OmniRoute's own usage stayed
trivial (confirmed live: 282MB/33GB at the time of the last trip).
Bump highPsiAvg10/criticalPsiAvg10/recoveryPsiAvg10 by 50% (20/40/10 ->
30/60/15); the ratio-based thresholds (this process's own OOM safety
margin) are unaffected.
@hartmark

hartmark commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the guard tripped again in production even with the faster-recovery fix, while OmniRoute's own container usage was trivial (282MB/33GB). Root-caused: /proc/pressure/memory reflects host-wide PSI, not this process's own cgroup pressure (confirmed by comparing it against /sys/fs/cgroup/memory.pressure from inside a running container — the two differ). On a shared host, contention from unrelated processes was enough to trip the guard.

Bumped highPsiAvg10/criticalPsiAvg10/recoveryPsiAvg10 by 50% (20/40/10 → 30/60/15) to tolerate that noise. The ratio-based thresholds (this process's own real OOM safety margin) are untouched. resource-pressure-policy.test.ts + resource-pressure.test.ts (16/16) and the complexity ratchet both pass.

@diegosouzapw
diegosouzapw merged commit d353870 into cheaper-inference:release/v3.8.51 Sep 3, 2026
16 checks passed
Bl0ck154 pushed a commit to Bl0ck154/OmniRoute that referenced this pull request Sep 19, 2026
… faster (cheaper-inference#12293)

Validado em lote numa worktree combinada com os 9 PRs desta leva sobre o tip de `release/v3.8.51` (já com a leva anterior dentro): os nove boardaram **sem um único conflito**, `typecheck:core` limpo e **80/80** nos 6 arquivos de teste que os PRs trazem.

O crescimento de arquivo próprio da leva foi rebaselinado num registro datado (`_rebaseline_2026_09_03_hartmark_batch`): `combos/page.tsx` 5012→5018 (cheaper-inference#12355, tratar o estado degradado quando o bundling de tiktoken de um provider sem relação falha) e `open-sse/services/combo.ts` 4023→4036 (cheaper-inference#12338, os fixes do universal-handoff). As violações restantes (`codex.ts`, `stream.ts`) foram medidas também no tip puro e são drift da base, não desta leva.

Obrigado, @hartmark.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… faster (cheaper-inference#12293)

Validado em lote numa worktree combinada com os 9 PRs desta leva sobre o tip de `release/v3.8.51` (já com a leva anterior dentro): os nove boardaram **sem um único conflito**, `typecheck:core` limpo e **80/80** nos 6 arquivos de teste que os PRs trazem.

O crescimento de arquivo próprio da leva foi rebaselinado num registro datado (`_rebaseline_2026_09_03_hartmark_batch`): `combos/page.tsx` 5012→5018 (cheaper-inference#12355, tratar o estado degradado quando o bundling de tiktoken de um provider sem relação falha) e `open-sse/services/combo.ts` 4023→4036 (cheaper-inference#12338, os fixes do universal-handoff). As violações restantes (`codex.ts`, `stream.ts`) foram medidas também no tip puro e são drift da base, não desta leva.

Obrigado, @hartmark.
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