Skip to content

server: separate resident sessions from active requests - #930

Open
JordiPosthumus wants to merge 2 commits into
antirez:mainfrom
JordiPosthumus:codex/server-max-active-requests
Open

server: separate resident sessions from active requests#930
JordiPosthumus wants to merge 2 commits into
antirez:mainfrom
JordiPosthumus:codex/server-max-active-requests

Conversation

@JordiPosthumus

@JordiPosthumus JordiPosthumus commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

--batched-session currently controls both resident KV allocation and the
number of requests admitted to inference. This patch adds
--max-active-requests N so those limits can be set independently.

For example:

./ds4-server --batched-session 10 --max-active-requests 1 ...

allocates ten resident slots while allowing one assigned or running request.
Additional requests remain queued until an active request completes or is
cancelled. Omitting the option preserves current behavior by setting the
active limit equal to the resident-session count. The option requires batched
mode, must be positive, and cannot exceed the resident-session count.

This is admission control only. It does not change which slot the existing
router selects. As discussed in #765, current upstream routing may reuse one
slot for unrelated sequential conversations even when other slots exist.
Consequently, this option alone does not guarantee that ten sequential
conversations remain warm; reuse-aware slot selection is complementary work.

The scheduler regression covers the default limit, a limit of one, worker
claim, completion, cancellation, and raising the limit to two. CLI rejection
tests cover invalid combinations. Forced Metal and CPU builds,
ds4_test --server, ds4_agent_test, the DeepSeek vision image test, the
server suite under ASan+UBSan, and focused DeepSeek-V4-Flash-Vision-Exp checks
pass on Apple M3 Ultra.

Current-main recheck — 2026-09-03

Applied independently to main b0a147a7fba6d1a104d047d5a140e9bb4bfc13cd in a clean
worktree. The default build, ds4_test --server, and ds4_agent_test pass.
The admission/routing distinction above is unchanged; the ten-conversation
integration result is not being attributed to this PR alone.

@kmike

kmike commented Sep 1, 2026

Copy link
Copy Markdown

ELI20: --batched-session 10 currently does two things at once: it allocates
ten independent KV sessions and allows up to ten requests into the inference
scheduler. Setting it to one removes compute contention, but also leaves room
for only one warm conversation, so unrelated conversations repeatedly evict
each other's KV. This patch splits those controls. The server can retain ten
conversation checkpoints in memory while assigning only one request at a time;
queued requests do not consume or evict a resident slot until they are actually
scheduled.

I think it's currently a bit more nuanced (or, rather, a bit more broken :). Even with --batched-session 10 the in-memory KV cache is only kept for 1 slot, if there is 1 request at a time (no parallel requests), due to broken logic of "which KV cache slot to use". On-disk KV cache can somewhat mask the issue, but it's unnecesary SSD trashing. See #765.

@JordiPosthumus
JordiPosthumus force-pushed the codex/server-max-active-requests branch from ba7b962 to 998fc19 Compare September 1, 2026 16:24
@JordiPosthumus

Copy link
Copy Markdown
Contributor Author

Agreed — slot selection is a separate upstream problem, and the earlier description overstated what this PR guarantees. #930 limits admission only; it does not fix the routing behavior discussed in #765.

I have updated the code documentation and PR description accordingly. With current upstream routing, --batched-session 10 --max-active-requests 1 allocates ten slots and limits compute to one request, but cannot guarantee that ten sequential conversations occupy ten different warm slots. A reuse-aware/staleness-aware routing fix is complementary work.

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