Pull requests / #2082
#2082 fix: make conversation parking admission fail safely
open · @ivanov84 · 0 コメント · GitHub で見る
Setup & installMulti-GPUAMD / HIPNVIDIA / CUDAModels & quantsSecurityWindows
本文
## Summary This hardens conversation-cache admission and retained K/V reuse under memory pressure. On Windows the engine runs out of the *system commit* limit rather than physical RAM, and the park path had no way to tell that it had already given up, no way to see why an allocation failed, and it silently dropped the retained K/V whenever admission said no. The eviction round now lives in `ConversationCache::evict_for_admission()`, which makes the loop's decisions testable for the first time; the tests assert the old and new loops against one cache state. ## Changes - **Stop eviction when it cannot make admission progress.** The inline loop was bounded by `slots()` and called `evict_oldest()` unconditionally, so a request larger than the total freeable capacity evicted every parked conversation and still refused. `evict_for_admission()` checks the request against `evictable_bytes()` first and stops at the first round that stops making progress, reporting why (`stalled`, `hopeless`, `freeable`). - **Avoid unnecessary eviction when no additional capacity is required.** The retained K/V is already held, so it is subtracted before the admission need is computed; a park that needs no new capacity evicts nothing. - **Preserve retained K/V reuse state when admission is declined.** `take_reuse()` used to leave the buffers with the caller, so a declined park lost them and the next turn rebuilt them. They go back through `put_reuse()` on every early-return path. - **Improve diagnostics.** Allocation failures now name where they happened (`at the capture`, `after the checkpoint split`, `after the full-model load`) and the probe reports whether the machine is out of commit or out of address space. Refusal lines print both resources and the process' own commit rather than one number. - **Regression coverage** for admission and eviction edge cases, including a twelve-case table asserting the old and new loops agree everywhere they should and differ only in the hopeless case. ## Validation Built and run on Windows, MSVC + CUDA 12.8, sm_120: ``` cmake -G Ninja + fresh build dir exit 0, 1 warning (upstream dequant.cpp C4456) conversation_cache_test 4286 checks passed conversation_memory_test 32 checks passed conversation_split_failure_test stages 1/2/3 allocation-failure cases preserved: 8/12/16 ctest 43/43 passed, 2 skipped (CPU-only build: expert_multi, isa_guard) cl /Zs over the CUDA park path exit 0 CUDA engine build exit 0, strata 0.1.43 git diff --check clean ``` String audit of the built engine: the textless `allocation failed; skip parking` is gone, and the named-cause and admission lines are present. End-to-end, against a real model on this machine — the user's production server running a build of this tree, 2026-10-11 23:42 to 2026-10-12 02:46, two RTX 5060 Ti with `--layer-split auto`, `--max-context 262144`, `--conversation-cache-mib 16384 --conversation-cache-slots 4 --conversation-cache-min-free-mib 4096`. Counted over that session's log: ``` park attempts: 216 (176 at >= 50k tokens, largest 232826 tokens) refusals: 1 of 216 "allocation failed; skip parking": 0 restores: 130; 129 of the 216 parks reused retained K/V (up to 3.29 GiB in one park) evict-to-admit rounds: 12 (11 evicted one conversation, one evicted two; largest admitted request 4006 MiB) budget (make_room) refusals: 0; refusal arms seen: hopeless 1, stalled 0, short_by 0 ``` The single refusal is the `hopeless` arm: the request was 3966 MiB against 2152 MiB freeable in total, and it names both numbers instead of evicting a conversation that could not have paid for it. Parking continued normally afterwards. The same workload on the unpatched 0.1.40.1 engine logged 293 `skip parking` lines — 203 of them the bare textless `allocation failed; skip parking` and 90 the budget refusal ending in a static `or telemetry unavailable` — out of 2772 attempts (291 refused, 10.5%). That build never ran an evict-to-admit round at all: the loop that would have run does not exist in 0.1.40. That run was built from the 0.1.42 base of this series; the rebase onto 0.1.43 changed no source of the patch, so the runtime evidence is the same code. The binary was string-audited first: the textless failure line absent, the named-cause and admission lines present. ## Limitations - The stalled arm of `evict_for_admission()`, the `bad_alloc` probe, and the budget-refusal K/V hand-back are asserted only by the native tests. The run above produced no stalled round (evicting did return memory on this machine), no failed allocation, and no budget refusal, so those three paths are runtime-untested. - Windows-only validation. The SYCL copy of the park path was edited by hand and reviewed, but no oneAPI toolchain is installed here, so it is not compiled. The header changes do reach the SYCL build through the shared include path. - Multi-GPU runtime behavior beyond this one two-GPU WDDM data point is not covered. - The change does not reduce the system-commit cost of the resident expert arena, which is the underlying pressure on this machine; it makes admission stop early, keep what it already holds, and say why it declined. - Not every previously observed parking failure has a known cause. What is measured here is that these refusals now explain themselves. ## Review focus - Retained K/V ownership on every early-return and exception path in the park body: it is taken with `take_reuse()` and must be handed back with `put_reuse()` on all of them. - The resource accounting used by admission: `evictable_bytes()` deliberately excludes the retained K/V and pinned entries, because `evict_oldest()` cannot free either. `bytes()` still charges both, which is correct for the cache budget and wrong for "what can I free right now". - The safety of the allocation diagnostic probe: it reserves, tries to commit, and must release the reservation on every path, including failure. - `ConversationCache::clear()` from #879 is upstream's, and it is consistent with this: it zeroes `reuse_`, and `evictable_bytes()` never counted it. <sub>This PR was prepared by an AI agent (OpenHands) on behalf of the author. The agent's work was reviewed by two independent review passes before the branch was pushed; the review notes and the answers to them are recorded in `LOCAL-PARKING-PATCH-0.1.43.md`, which is included in the branch along with the repo-local `AGENTS.md` notes. Happy to drop either file from the change if you would rather keep them out of the repository.</sub>
関連リンク
インストール・モデル・リリースへの站内リンク。