[release-1.0] Add session boost queue wait timeout (HTTP 504 on excessive wait)#1327
Conversation
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
FAIRNESS_QUEUE_TIMEOUT now applies only to the user-fairness queue. In session-boost mode the request context no longer carries the fairness deadline; the only server-side queue-wait bound is the optional SESSION_BOOST_MAX_WAIT (429). Docs updated accordingly. Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com> Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Realign markdown config tables after the FAIRNESS_QUEUE_TIMEOUT note length change. No content change. Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com> Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
…s, isolate test Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
…IMEOUT Address hzxuzhonghu's review comments: - Remove SESSION_BOOST_WAIT_REJECT_ENABLED; the timeout is enabled implicitly and controlled solely by SESSION_BOOST_TIMEOUT (default 30s, non-positive disables). - Rename SESSION_BOOST_MAX_WAIT to SESSION_BOOST_TIMEOUT to align with FAIRNESS_QUEUE_TIMEOUT. - Apply the timeout to the request context via context.WithTimeout and reuse the existing reqCtx.Done() case instead of a separate timer/select branch. - Use a strategy-aware log prefix (SessionBoost vs FairnessScheduling) in the shared handler. - Update Helm charts/templates, docs, proposal, and tests accordingly. Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a queue wait timeout (SESSION_BOOST_TIMEOUT) for the session-boost scheduling strategy in the Kthena router, defaulting to 30 seconds, to prevent requests from waiting indefinitely under heavy load. The implementation safely handles races between dequeue admission and request abandonment/timeout to prevent permit leaks, and includes updates to Helm charts, configuration values, documentation, and unit tests. The reviewer's feedback correctly identifies an inconsistency in the proposal documentation regarding whether this timeout is enabled or disabled by default.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 3. **KV cache optimization**: Prioritize follow-up requests from recently completed sessions to maximize warm cache hits. | ||
| 4. **Grace period (advanced, off by default)**: An optional, tricky tuning knob that, after a request completes, briefly holds the dequeue slot for a potential follow-up from the same session before dispatching unrelated requests. It is **disabled by default** and should only be enabled by operators who fully understand that it deliberately delays unrelated requests in exchange for a higher same-session prefix-cache hit rate. | ||
| 5. **Backpressure-aware**: Respect backend pod capacity to avoid flooding, using two-level admission control (inflight limit + backend metrics). | ||
| 6. **Fail-fast queue wait timeout (optional, off by default)**: Optionally reject requests that wait in the queue longer than a configurable threshold with HTTP 504, so latency-sensitive clients shed load or retry instead of waiting indefinitely for backend capacity. `FAIRNESS_QUEUE_TIMEOUT` governs only the user-fairness queue and does not apply in session-boost mode. |
There was a problem hiding this comment.
There is an inconsistency in the documentation regarding the default state of the queue wait timeout. Point 6 states that the timeout is "off by default", whereas the configuration table and other sections of the documentation (as well as the Helm values and code implementation) correctly state that it is "enabled by default" at 30s. Please update this point to reflect that it is enabled by default to avoid confusing users.
| 6. **Fail-fast queue wait timeout (optional, off by default)**: Optionally reject requests that wait in the queue longer than a configurable threshold with HTTP 504, so latency-sensitive clients shed load or retry instead of waiting indefinitely for backend capacity. `FAIRNESS_QUEUE_TIMEOUT` governs only the user-fairness queue and does not apply in session-boost mode. | |
| 6. **Fail-fast queue wait timeout (optional, enabled by default)**: Optionally reject requests that wait in the queue longer than a configurable threshold with HTTP 504, so latency-sensitive clients shed load or retry instead of waiting indefinitely for backend capacity. `SESSION_BOOST_TIMEOUT` (default `30s`) bounds the wait; set a non-positive duration to disable it. `FAIRNESS_QUEUE_TIMEOUT` governs only the user-fairness queue and does not apply in session-boost mode. |
There was a problem hiding this comment.
Pull request overview
Adds an independent queue-wait timeout for session-boost scheduling so requests don’t wait indefinitely under sustained overload, returning HTTP 504 after SESSION_BOOST_TIMEOUT (default 30s) while ensuring fairness-mode timeout behavior remains controlled solely by FAIRNESS_QUEUE_TIMEOUT.
Changes:
- Router: introduces
SESSION_BOOST_TIMEOUTand applies it only in session-boost mode via request-scoped context deadlines; improves strategy-aware logging and usesRequest.Abandon()on cancellation/timeout. - Datastore queues: hardens admission vs cancellation races (commit/admit vs abandon) to prevent inflight-permit leaks; adds targeted race-focused tests.
- Helm + docs: wires
networking.kthenaRouter.sessionBoost.timeoutinto router env and documents the new behavior across user guides and references.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/kthena-router/router/router.go | Adds session-boost timeout parsing/config and applies strategy-specific queue-wait deadlines (504 on deadline exceeded). |
| pkg/kthena-router/router/router_test.go | Extends handler tests to cover session-boost queue-wait timeout behavior (504). |
| pkg/kthena-router/datastore/session_boost_queue.go | Makes session-boost admission skip already-abandoned requests to avoid leaking inflight permits. |
| pkg/kthena-router/datastore/session_boost_queue_test.go | Adds tests for abandon/admit race scenarios (including concurrent stress). |
| pkg/kthena-router/datastore/fairness_queue.go | Introduces Request.commitAdmission()/Request.Abandon() to serialize admission vs abandonment and prevent permit/metric leaks. |
| docs/proposal/session-boost-strategy.md | Documents the session-boost queue-wait timeout design and configuration. |
| docs/kthena/docs/user-guide/session-boost.md | Updates user guide with timeout behavior, config examples, and operational guidance for 504s. |
| docs/kthena/docs/user-guide/fairness-scheduling.md | Clarifies FAIRNESS_QUEUE_TIMEOUT applies only to fairness scheduling (not session boost). |
| docs/kthena/docs/reference/helm-chart-values.md | Documents the new Helm value networking.kthenaRouter.sessionBoost.timeout. |
| charts/kthena/values.yaml | Adds chart default for networking.kthenaRouter.sessionBoost.timeout. |
| charts/kthena/charts/networking/values.yaml | Adds subchart default and description for session-boost timeout value. |
| charts/kthena/charts/networking/templates/kthena-router/component/deployment.yaml | Injects SESSION_BOOST_TIMEOUT env var into the router Deployment when configured. |
| charts/kthena/charts/networking/README.md | Documents kthenaRouter.sessionBoost.timeout chart parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 3. **KV cache optimization**: Prioritize follow-up requests from recently completed sessions to maximize warm cache hits. | ||
| 4. **Grace period (advanced, off by default)**: An optional, tricky tuning knob that, after a request completes, briefly holds the dequeue slot for a potential follow-up from the same session before dispatching unrelated requests. It is **disabled by default** and should only be enabled by operators who fully understand that it deliberately delays unrelated requests in exchange for a higher same-session prefix-cache hit rate. | ||
| 5. **Backpressure-aware**: Respect backend pod capacity to avoid flooding, using two-level admission control (inflight limit + backend metrics). | ||
| 6. **Fail-fast queue wait timeout (optional, off by default)**: Optionally reject requests that wait in the queue longer than a configurable threshold with HTTP 504, so latency-sensitive clients shed load or retry instead of waiting indefinitely for backend capacity. `FAIRNESS_QUEUE_TIMEOUT` governs only the user-fairness queue and does not apply in session-boost mode. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is an automated cherry-pick of #1277