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>

関連リンク

インストール・モデル・リリースへの站内リンク。