* Studio: admission control on /v1/messages, slot pool that tracks --parallel
/v1/chat/completions was gated by the llama admission queue but /v1/messages
was not, so an Anthropic client could oversubscribe llama-server's slots and
stall the backend. Wire the same queue into all six /v1/messages dispatch
sites, and rework the queue itself into an explicit slot pool.
- Queue keyed by base_url, so both API surfaces share one pool of slots.
- Waiting is unbounded by default instead of timing out; the wait line is
sized at 16 x the serving slots so it follows --parallel.
- Neutral UNSLOTH_LLAMA_ADMISSION_* env names, legacy UNSLOTH_OPENAI_COMPAT_*
spellings still honored.
- Passthrough retries once against a respawned llama-server, which comes back
on a new ephemeral port.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix over-admission on capacity shrink and restore the stream cancel contract
Review of the previous commit turned up two real regressions plus smaller gaps.
- Pool sizing looked only at free slot ids, so when capacity shrank while slots
were held (an unload resets effective_parallel_slots to 1) a freed low id was
handed out even though the holdovers already met the new ceiling. Count every
held slot against capacity instead. A 1-slot backend could run 4 generations.
- The streaming wrapper closed the monitored body with aclose(), delivering
GeneratorExit where _SameTaskStreamingResponse deliberately throws
CancelledError. The monitor entry was never finalized, so it leaked as
"running" for the process lifetime and cancel_event was never set. Close
through the shared helper so cancellation reaches the handler.
- Finalize the monitor when a stream is abandoned before its body starts, and
when a queued non-streaming request is cancelled (which also leaked the
un-awaited generation coroutine).
- Floor the scaled wait line at 64, so a 1-slot backend keeps the depth it had
before scaling existed instead of dropping from 64 to 16.
- Use the canonical Anthropic type map: a full queue is 429 rate_limit_error,
which SDKs back off on; overloaded_error is 529.
- Treat non-positive max_queue/queue_per_slot as unbounded rather than "reject
everything", and reclaim the slot if a waiter's event loop is gone.
Tests: regression tests for both defects, verified to fail without the fix.
Adds env coverage for QUEUE_PER_SLOT and the legacy fallbacks, a structural
check that all six dispatch sites stay admission-wrapped, and clears the new
env var in the isolation fixtures.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Run the response pre-start cleanup when a queued stream is abandoned
From automated review of the earlier commits.
_anthropic_passthrough_stream enters its _TrackedCancel eagerly and relies on
the stream's finally to exit it, but aclose() on an async generator that never
started is a no-op, so that finally never runs. Admission made this reachable:
a client that disconnects while queued leaves the cancel id registered in
_CANCEL_REGISTRY forever.
- Give the passthrough response an unstarted_cleanup hook that exits the
tracker, via a new optional arg on _sse_streaming_response.
- Chain to that hook from the admission wrapper rather than replacing it, and
run it when the wrapper gives up before the body started.
- Defer to an in-progress MTP fallback instead of respawning underneath it;
only the first caller gets True from _maybe_recover_from_mtp_crash.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Restore Python 3.9 support, and stop the floor overriding an explicit setting
Second review round. The first item is a real break shipped by the earlier
commits, the rest are correctness and contract fixes.
- dataclass(slots = True) and int.bit_count() are both 3.10+, but the package
declares requires-python >=3.9 and CI only runs 3.12, so nothing caught it.
Importing the module raised TypeError on 3.9, taking down the whole backend,
not just admission. Drop the dataclass slots and track the popcount in a
counter. A test now asserts neither API comes back.
- The queue-depth floor applied even when an operator set QUEUE_PER_SLOT
explicitly, so asking for a shallow line silently got 64 and, with no queue
timeout, callers blocked instead of failing fast. The floor now only backs
the default multiplier.
- Never let a failing close strand a slot: closing runs in its own try so the
release always happens. A lost slot shrinks the pool permanently.
- Close the generation coroutine when reserving fails for any reason, not only
on a full queue.
- Exit the passthrough cancel tracker if the client drops while the opening SSE
lines are still being sent; those yields sit outside the teardown try.
- snapshot.free now reports what a caller could actually take, so the admission
log cannot show free slots next to queued requests after a shrink.
- Correct the class docstring: the wait line is bounded by default, not
unlimited. Document that abandoning wait() requires cancel(), and pin the
thread assumption in _deliver_lease.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Make the leak guards real, and cover the untested admission branches
Third review round, which attacked the previous round's tests by reverting each
fix. Two guards turned out to be hollow.
- The pre-start cleanup chain could be severed with the suite still green: the
existing test drove the generator finally, never the response hook. A real
pre-start disconnect leaked the passthrough cancel tracker permanently.
Replaced with a test that runs the response hook and asserts _CANCEL_REGISTRY
is empty; verified against both ways of reintroducing the leak.
- The structural check only asserted the unstarted_cleanup keyword was present,
so passing a literal None passed it while leaking. It now asserts the hook is
actually built.
- test_shares_queue_with_openai_by_base_url never touched the OpenAI helper; it
was a duplicate under a misleading name. It now reserves through the same
helper /v1/chat/completions uses, so it fails if either surface ever derives a
different key. That is the PR's central shared-queue claim.
- Cover the passthrough dispatch site, 499 on disconnect-while-queued, and the
streaming admission timeout. Four of six sites previously had only an AST
node count behind them.
- Clear admission env in the autouse fixture rather than per test: an ambient
canonical name silently beat the legacy name a test was exercising.
- Loosen the wall-clock assertion, which guarded against serialising on the
uncontended path, not against a slow runner.
- The class docstring claimed a global concurrency cap; Studio's own chat
endpoint does not reserve, so it is not one.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Keep dataclass slots on 3.10+ via a version gate
Dropping slots = True for 3.9 gave it up everywhere, including the 3.12 CI runs
and every supported interpreter but one. Gate it instead: _SLOTS is
{"slots": True} on 3.10+ and empty below, unpacked into each dataclass.
The AST scan now requires the unpack rather than merely forbidding a literal
slots keyword, so a dataclass added later cannot quietly lose slots. Added a
test that the gate matches the running interpreter, since a gate that never
applies is worse than no gate. Verified the 3.9 branch by forcing _SLOTS empty
and reloading: the full admission suite passes either way.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix two slot leaks, and cover the guards that had no test
Third review round, three reviewers working independently on the admission core,
the route wiring, and whether the PR regresses anything it is not about.
Leaks:
- cancel() made the same call_soon_threadsafe as _grant_waiters_locked but
without its RuntimeError guard. Routes cancel from finally blocks, so a closed
loop masked their exception and skipped the release, stranding the slot and
pinning is_idle() false so the queue was never evicted either.
- The pre-start cleanup released the slot after an await that can raise
BaseException, which is swallowed upstream. Nested it in a finally, as the
streaming and OpenAI paths already do.
An unparseable QUEUE_PER_SLOT dropped the 64 floor while falling back to the
default multiplier, quietly giving a 1-slot backend a 16-deep line. Explicit now
means it parsed.
Guards that were correct but had no test. Each was reverted, confirmed the suite
stayed green, then covered and confirmed red:
- the slot released when stream setup raises, which is the reachable one:
count_chat_tokens is a blocking call to llama-server, so a dead backend raises
after the slot is taken and before a body exists to release it
- coro.close() on a cancelled queued request, the api_monitor.fail that
distinguishes an admission timeout from a client hang-up, the MTP fallback
short-circuit, and the BaseException guard around the opening stream lines
- the queue-full test asserted a type string OpenAI's 429 also uses, so it
passed against an OpenAI envelope. It now pins the Anthropic shape.
Anthropic requests were invisible in the admission log while sharing the pool
with chat completions, so the same events are logged there with a mode. Renamed
the helper to match, since it is no longer OpenAI-only.
Corrected two comments that described behaviour the code does not have: the
slot is taken when the streaming response is built, not when the body starts
iterating, and the pool is not a cap on every generation, since /v1/completions,
Studio's chat endpoint and RAG captioning all reach llama-server directly.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Cover the admission telemetry, and drop a dead helper
Fourth review round. No bugs found in the code this time; the finding was that
most of the previous commit's telemetry had no test. Only queue-full was
asserted, so removing any of the other four log calls left the suite green.
All five are covered now, each verified by removing its call and confirming only
its own test reds. Fixing the first attempt turned up a test bug of my own: the
log line carries a queued=N field, so asserting "queued" in the message matched
every admission log ever emitted. It asserts the event name now.
Also covered two guards that were correct but unguarded: waiters whose futures
die out of band stop counting against the queue depth, and a newcomer cannot
barge past a parked waiter. The second is pinned as behaviour rather than as the
`if not self._waiters` check, because that check cannot actually change the
outcome: _take_slot_locked consults _can_admit_locked anyway, so either alone
refuses the newcomer. The test fails only if both go.
_optional_positive_int_env lost its last caller when the env parsing was
rewritten last round. Removed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2ab4b744ac