Pull requests / #309

#309 serve: keep retained K/V through the cache capacity gate

closed · @chimpera · 0 コメント · GitHub で見る

Server & API

本文

Follow-up from running the conversation cache (#189) in production and post-morteming a long real-world session. Nothing here is a correctness bug — the cache stayed consistent throughout — it's an efficiency cliff the capacity gate creates once the cache is full.

**What.** `park_current` discards the retained K/V whenever `can_fit` fails and falls back to a full snapshot capture:

```cpp
if (!reuse.kv.empty() && !conversations.can_fit(estimate, held)) {
    // Optional growth capacity must not evict useful conversations.
    reuse = {};
    estimate = fresh_estimate;
}
if (!conversations.make_room(estimate, held)) { /* skip parking */ }
```

This removes the gate. Capacity stays `make_room`'s decision — it runs immediately after — and `put()`'s accounting still bounds the budget.

**Where it bites.** The workload is a serving box with one long-lived conversation re-parked every round trip (an agent tool loop) while short-lived conversations come and go and keep the cache full. Once `bytes()` is at budget, the gate fires on EVERY park of the long conversation: the restore before each park had retained its K/V correctly, and the park throws it away. The entry still parks — as a whole-snapshot capture (hundreds of ms at ~100K tokens; the incremental path exists for exactly this shape and never engages), and the cycle repeats every round trip.

**Why the fallback doesn't protect the thing it claims.** The comment says growth must not evict useful conversations, but `make_room` enforces that policy itself, with the priority your tests pin (conversation_cache_test.cpp:80-82):

```cpp
if (bytes() > budget_ - held - incoming) reuse_ = {};   // drop optional storage first
while (!entries_.empty() && (entries_.size() >= slots_ || bytes() > budget_ - held - incoming)) { ... }
```

So a park carrying its own retained K/V is not a fresh arrival competing for capacity: the parked image *replaces* memory the cache already counts through `reuse_`, and under real pressure `make_room` drops the reuse before evicting anyone. Once the cache is full, the gate changes only WHAT gets captured, and both of its terms are covered downstream anyway:

- the slot term: `make_room` also evicts while `entries_.size() >= slots_`, so with slots full exactly one LRU eviction happens whether the reuse was kept or dropped;
- the byte term: the fallback's `fresh_estimate` is LARGER than the with-reuse estimate, so `make_room` evicts the same entries or MORE.

Net effect of the gate at a full cache: identical-or-worse eviction, plus a whole-snapshot recapture instead of the growth. If we're misreading the intent behind the pre-check we'd genuinely like to know — but as far as we can trace, nothing it protects survives its removal.

**One subtlety reviewers should check.** The with-reuse estimate passed to `make_room` must stay UNCAPPED. `ConversationBuffer::bytes()` counts segment *capacities*, and `put()` charges the completed image's true size on the same basis — capping the estimate at the fresh figure would under-evict and overfill the budget. (We found this the hard way with a capped variant: a small-budget run parked an image `make_room` had not made room for.)

**Tests.** None needed: `can_fit` stays as a non-mutating query with its existing coverage (the unit tests at conversation_cache_test.cpp:76-82 still pass unchanged). On top of the unit suites we ran the repo's own conversation_cache_parity harness on this exact change (single GPU, STRATA_STATE_HASH): reuse at spec 1 byte-exact and spec 4, the pressure scenario at a fit-individually-not-together budget, and the exchange scenario — the last on both the patched and unpatched build at the same budget, identical PASS. A serve-loop-level regression test for "a full cache keeps the incremental re-park" would need the two-conversation harness; happy to add one if you want it in-tree.

**Measured** on our serving deployment (2-GPU split, int8 K/V, ~300 KB/token parked): a ~160K-token conversation with the cache at budget re-parks in growth-only time instead of a 400-900 ms full capture per tool round trip; restores are unaffected (56 ms for a 64K-token checkpoint).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

関連リンク

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