Repository navigation
fix(token_counter): stop large token counts from blocking the proxy event loop - #37697
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThe PR bounds tiktoken encoding work and moves token counting off asynchronous proxy and router event-loop paths.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported event-loop counting paths, invalid chunk-size handling, and heterogeneous-deployment fallback have been addressed.
|
| Filename | Overview |
|---|---|
| litellm/constants.py | Defines a bounded token-encoding chunk size, resolving the previously reported invalid-value behavior. |
| litellm/litellm_core_utils/env_utils.py | Adds integer environment parsing with inclusive range validation and safe fallback. |
| litellm/litellm_core_utils/token_counter.py | Encodes tiktoken input in bounded chunks to avoid pathological single-call merge costs. |
| litellm/proxy/proxy_server.py | Runs fallback token counting in a worker thread rather than on the proxy event loop. |
| litellm/router.py | Precomputes context-window counts off-loop, scans deployments independently for declared limits, and prevents inline recounting after failure. |
Reviews (3): Last reviewed commit: "fix(token_counter): stop large token cou..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2efbaf9 to
8c007b7
Compare
|
@greptileai re-review at 8c007b7: counts now run off the event loop in a worker thread, and the chunk-size setting is bounded and validated |
8e88401 to
3568a21
Compare
…vent loop tiktoken's BPE merge loop is quadratic in the length of a single regex piece, so a long run of one repeated character turns a multi-MB payload into minutes of CPU. Encoding in bounded chunks makes that linear, at a drift of at most ~1 token per chunk boundary. Chunking alone only makes the stall shorter, so the async paths now count in a worker thread: tiktoken releases the GIL for its Rust encode, so the loop keeps serving other requests while a count is in flight. The /utils/token_counter endpoint awaits the new atoken_counter, and the router's async deployment selection counts off-loop and hands the result to _pre_call_checks instead of making it count inline. The chunk size knob is bounded to [1, 4096]: a non-positive value used to raise or silently report zero tokens, and an arbitrarily large one restored the quadratic cost this exists to remove. Out-of-range and unparseable values warn and fall back to 1024.
3568a21 to
db68877
Compare
|
@greptileai re-review at db68877: the off-loop count no longer falls back to an inline on-loop recount, and unmappable deployments no longer hide a declared limit |
tin-berri
left a comment
There was a problem hiding this comment.
Foundational and billing-adjacent (token counting feeds context-window pre-filtering and cost estimation) so dispatched a deep review that empirically re-tested the claims against real tiktoken rather than trusting the PR's own numbers. Findings: the chunk-size clamp [1, 4096] with fallback-to-1024 genuinely fixes two real pre-existing bugs — chunk_size=0 previously raised ValueError from range(), and a negative value silently produced zero tokens via an empty range, which would have quietly disabled every downstream context-window/trimming/accounting decision — both edge cases are now clamped correctly and tested. The off-loop async offload (via the existing asyncify helper) and the new skip_inline_token_count flag are sound: no double-counting (input_tokens is seeded once, the inline branch only fires when the precomputed value is None), and a failed/None off-loop count falls through to the exact same 'return the full unfiltered deployment list' path the pre-existing exception handler already used — not a new degradation mode. One nuance worth a note: the disclosed ~0.06% chunking drift isn't strictly guaranteed to over-count — empirical testing found ~2% of cases under-count by 1-2 tokens due to tiktoken's regex pre-tokenizer splitting differently at a chunk boundary. The magnitude is immaterial for budget/context-window enforcement, but the PR's caveat could be a touch more precise about direction not being guaranteed. Two minor non-blocking nits: get_router_model_info is now called twice per deployment in the pre-call-check path, and the PR's own body honestly discloses a third _pre_call_checks-adjacent call site (the cost-calculator success handler) that remains on-loop, bounded only by chunking (~0.5s) — worth a tracked follow-up if one doesn't exist. Proof-of-fix is dramatic and real (live proxy, 46s->4.8s with actual Anthropic billing, all 150 liveness probes passing vs most timing out/refused before). CI green aside from the long-running benchmarks job.
|
Heads-up: this moves |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a developer sending a large multimodal agent request gets it served, but the gateway stops answering anything else for the better part of a minute, and the replica gets pulled out of rotation
context deadline exceeded, CPU pinned at exactly 1.0 vCPU, no OOM and no restart, and unrelated in-flight calls surface boguslitellm.Timeout: Connection timed out. Timeout passed=30.0, time taken=0.001 secondserrorslitellm_settings: disable_token_counter: true, which gives up token-based context-window pre-filtering and makes POST https://litellm-domain/v1/messages/count_tokens return 503 whenever the provider's own count API failsAfter: the same request is served promptly and the replica keeps answering everything else while it is in flight
disable_token_counterworkaround is neededRelevant issues
Linear ticket
Resolves LIT-5426
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup: a live proxy on port 4010 with
router_settings: {enable_pre_call_checks: true}and one direct Anthropic deployment, so the router's context-window pre-call check runs on the request path:Cases 2 and 3 each send one request in the background and fire 150 liveness probes at 0.2s intervals while it is in flight:
Before (cde1344)
Case 1: SDK token counting (the ticket's own repro)
Case 2: liveness probes while one large POST /v1/messages is in flight
http://localhost:4010/v1/messageswithpayload2.jsonCase 3: liveness probes while one large POST /utils/token_counter is in flight
http://localhost:4010/utils/token_counterwithpayload3.jsonAfter (db68877)
Case 1: SDK token counting (the ticket's own repro)
Case 2: liveness probes while one large POST /v1/messages is in flight
http://localhost:4010/v1/messageswithpayload2.jsonCase 3: liveness probes while one large POST /utils/token_counter is in flight
http://localhost:4010/utils/token_counterwithpayload3.jsonType
🐛 Bug Fix
Review notes
What this covers, and what it does not
LIT-5426 names three call sites. Two are now genuinely off the event loop and this PR solves them: the router context-window pre-call check, and the local fallback behind
/utils/token_counterand/v1/messages/count_tokens. The proof above measures exactly those: 150 of 150 liveness probes answering at a 1.0ms median where the merge base managed 129 of 150 at a 25.27s median with 21 refused outright.The third, the cost-calculator fallback that runs when a response carries no usage, is mitigated rather than solved.
_async_success_handler_bodycalls_success_handler_helper_fndirectly, with no executor and no thread hop, so that count still runs on the loop. Chunking takes it from over five minutes (killed) to 0.49s on a 2.4MB single-character payload, which is bounded and linear but not zero. That 0.49s is a bench number for the encode itself, not a proxy measurement: unlike the two cases above, I did not instrument that path against a live proxy.I did not take the third because moving it off-loop means making the async logging path async through
completion_cost, which reaches well beyond token counting and belongs in its own changeWhy the encode moved off the loop
Chunking on its own only shortens the stall. The whole chunk loop still ran on the event loop thread, so a big enough payload kept monopolizing it, which is the ticket's actual premise. Measured on this machine, a single
encodeof"A" * 200000blocks the loop for 8.26s: a competing coroutine's largest scheduling gap is 8263ms. Running the same encode through a worker thread leaves the max gap at 2.66ms and lets the competitor run 587k times, which confirms tiktoken releases the GIL for its Rust encode and that a thread offload genuinely helps hereSo the async paths now count off-loop, through the repo's existing
asyncifyhelper./utils/token_counterawaitsasyncify(litellm.token_counter), resolved at call time so the public seam callers and tests already patch keeps working, and the router'sasync_get_healthy_deploymentscounts off-loop and passes the result into_pre_call_checksrather than letting it count inline._pre_call_checkskeeps its own lazy inline count for the sync entry points, but the async caller setsskip_inline_token_count, so a count that fails off-loop is never retried on the loop; it just leaves the deployments unfiltered, which is what a failed count already did. For the same reason the pre-count resolves each deployment's context window exactly the way_pre_call_checksdoes, skipping ones it cannot map rather than letting the first unmappable deployment hide a limit declared by a later one. Sync callers such ascompletion_costare unchanged: there is no loop to protect there, and chunking already bounds their costWhy the chunk size is bounded on both ends
The lower bound closes more than a wrong number. A configured
-1produced no chunks at all and silently reported zero tokens, and a zero count is not cosmetic: every downstream consumer of it, the context-window pre-call filter and the budget and rate-limit accounting, then sees a free request, so a single mistyped environment variable quietly stopped the proxy enforcing its limits. The clamp closes that as well as the performance holeThe chunk size still needs a ceiling even with the offload, because the cost is linear in the chunk size: on a 2.4MB single-character payload it is 0.28s at 512, 0.49s at 1024, 1.8s at 4096 and 13.3s at 16384.
TIKTOKEN_ENCODE_CHUNK_SIZE_CHARSis therefore clamped to [1, 4096]. Zero used to raiseValueErrorfromrange()and a negative value produced no chunks at all, silently reporting zero tokens and quietly disabling every context-window, trimming and accounting decision downstream. Out-of-range and unparseable values now warn and fall back to 1024Caveats (if any)
imageblocks still break counting, ticketed separatelyFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/e1ccb43093544b83b0bebcf711acd91b
Requested by: @yassin-berriai