feat(config): deployment-wide default for upstream request/stream timeouts - #844
Conversation
…eouts A Model that set neither timeout nor stream_timeout previously had no deadline at all: an upstream that accepted the connection and then went silent forever held the request open indefinitely (TCP keepalive cannot detect a live-but-silent peer). The reference LLM proxies never run unbounded — their router falls back to a global request_timeout (6000 s) when no deployment-level timeout is set. - New upstream.timeout_ms (default 6_000_000 = 6000 s) and upstream.stream_timeout_ms (default 0 = fall back to timeout_ms). - Deadlines now resolve target -> group -> deployment default via routing::effective_timeouts, mirroring effective_retries. The group Model's timeout/stream_timeout used to be dead fields for members; they are now the middle resolution level. - Explicit `timeout: 0` at model or group level opts that resource out of the backstop; `upstream.timeout_ms: 0` restores the old behaviour deployment-wide. - The deployment-default streaming budget deliberately does NOT arm the pre-200 first-chunk peek (TimeoutBudget::stream_configured): peeking withholds the 200 until the first token, which would disable the SSE heartbeats (#818) for every model. Resource-level budgets keep the slow-first-token failover (#554) exactly as before. - Applied uniformly across chat, messages, responses, count_tokens, completions, embeddings, rerank, images, audio, videos, jobs, passthrough, ensemble (panel + judge) and the realtime idle cap. Behavioural changes for existing deployments: 1. Models with no timeout configured anywhere are now bounded at 6000 s. 2. Group-level timeout/stream_timeout now apply to members that do not set their own.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
Comment |
…doc links Audit follow-ups on the deployment-timeout-defaults change: - The Model.stream_timeout field doc (and thus the generated schema description) contradicted the implementation: the resource-resolved timeout outranks the deployment stream default, not the other way round. The docs now describe the implemented order. - effective_timeouts' rustdoc over-promised that timeout: 0 always opts the stream out — a group-level stream_timeout still supplies a budget (the dedicated knob outranks the generic one at every level, which is also how the LiteLLM router resolves it: the stream chain is exhausted before the non-stream chain starts). Two unit tests now pin those cross-level corners. - Two rustdoc links still pointed at the removed Model::request_timeout.
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Problem
A Model that sets neither
timeoutnorstream_timeouthas no upstream deadline at all: an upstream that accepts the connection and then goes silent forever holds the request open indefinitely. TCP keepalive (already on by default) only detects a dead peer — it cannot detect a live-but-silent one, so the only bound left is the client's own patience. The mainstream LLM-proxy baseline never runs unbounded: its router falls back to a globalrequest_timeoutof 6000 s when no deployment-level timeout is set, and its per-deploymentstream_timeoutfalls back to the non-stream timeout the same way.Two smaller gaps in the same area:
timeout/stream_timeoutfields on a model group (a Model carryingrouting) were dead fields — members only ever used their own values, unlikeretries, which already resolves member → group → deployment default.Change
upstream.timeout_ms(default6_000_000= 6000 s, matching the reference proxy'srequest_timeout) andupstream.stream_timeout_ms(default0= fall back totimeout_ms). Env overrides work like every otherupstream.*knob (AISIX_UPSTREAM__TIMEOUT_MS,AISIX_UPSTREAM__STREAM_TIMEOUT_MS).routing::effective_timeoutsresolves deadlines target → group → deployment default, mirroringeffective_retries, and is applied uniformly across every dispatch family: chat, messages (passthrough + cross-provider), responses (both), count_tokens, completions, embeddings, rerank, images, audio (transcription + speech), videos, jobs, passthrough, ensemble (panel + judge), and the realtime idle cap.timeout: 0at model or group level opts that resource out of the backstop (unchanged for existing users of0);upstream.timeout_ms: 0restores the old unbounded behaviour deployment-wide.stream_timeout: 0keeps its historical "defer" semantics at every level.stream_timeout, then the resource-resolvedtimeout), deployment defaults last. Within that, the dedicated stream knob outranks the generic one at every level — a group'sstream_timeoutbeats a member's owntimeoutfor the streaming budget, and supplies one even to a member whosetimeout: 0opted out of the request deadline (unit-tested; same shape as the reference router, whose stream chain is exhausted before its non-stream chain starts).TimeoutBudget::stream_configured). Peeking withholds the 200 until the first token — that is the /v1/completions: add output guardrails on generated choices[].text #554 slow-first-token failover contract and stays exactly as-is for resource-configured budgets — but doing it for the deployment default would withhold headers for every model and silence the feat(downstream): configurable idle timeout and SSE heartbeat on the inbound side #818 SSE heartbeats whose whole purpose is covering a slow first token. With only the default budget, a first-chunk stall surfaces as an in-band timeout after the 200 instead of failing over. Same shape asRetryBudget::covers: explicit config opts into the sharper behaviour.Behaviour changes
timeoutconfigured anywhere are now bounded at 6000 s end-to-end (non-streaming) / 6000 s inter-chunk (streaming). This is a backstop against infinite hangs, deliberately generous so it can never cut a legitimate long request.timeout/stream_timeoutnow apply to members that don't set their own. A pre-existing group-levelstream_timeout(previously a dead field) therefore also arms the pre-200 first-chunk peek for members without their own streaming config.timeout: 0to unset — after this change those models pick up the 6000 s backstop instead of staying unbounded. A companion CP PR (preserve explicit0, expose the group-level fields) is queued to restore the opt-out there; standalone/file-based deployments are unaffected.Tests
routing::effective_timeoutsunit tests: three-level precedence, explicit-zero opt-out at each level, stream fallback order, peek arming.upstream-timeout-defaults-e2e.test.ts(real binary + etcd + mock upstream): default applies (504 at the deadline),timeout: 0opt-out, explicit model timeout beats the default, streaming inherits the default as its chunk budget without suppressing the 200/heartbeats, group timeout applies to members.timeout-fallback-e2e(the /v1/completions: add output guardrails on generated choices[].text #554 contract) anddownstream-connection-e2e(the feat(downstream): configurable idle timeout and SSE heartbeat on the inbound side #818 heartbeat contract) pass unchanged.timeout/stream_timeoutschema delta is description-only).