Repository navigation
feat(gateway): enforce request deadlines, cancellation, and bounded shutdown - #2538
Conversation
Keep the original request deadline and model permit through local HTTP/1 delivery and owner termination. Observe disconnects without rewriting completed execution or model-send facts, while preserving fallback quotas and keep-alive. Include controlled pending-write and real TCP regressions, spawned Gateway/Python Core UDS/local-provider acceptance, and bilingual deployment documentation for gateway-g3-follow-up issue 02.
Reject malformed HTTP and UDS chunk framing before recovery retries. Prioritize cancellation and deadlines over pending response writes and track model-content delivery only for released or degraded responses. Add regression coverage and align English and Chinese deployment docs with connection pooling and bounded shutdown behavior. Validation: Gateway workspace tests (191 passed), four diagnostic scenarios, cargo fmt, and Clippy with warnings denied. Full website link checking was blocked by PyPI dependency timeouts.
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
508d909 to
5fb2b2d
Compare
hidb4ai
left a comment
There was a problem hiding this comment.
CI readiness note for 7d9dc847bc21b88b8b8260a4634e9334c8198c75:
- OpenDAL packaging fails with
pydantic-discarded-extra-argumentatintegrations/opendal/src/powercontext_connector_opendal/source.py:160. The base commit's run has the same diagnostic, confirming a baseline failure. - SQLite acceptance and OceanBase acceptance both fail before agent startup: their retained
agent/acp.txtreportsImportError: cannot import name 'text_block' from 'acp'in/installed-agent/acp_runner.py. The runner and its dependency setup are outside this diff.
Please resolve these shared CI/dependency failures and rerun the checks before merging. These are merge-readiness blockers; the evidence does not establish a defect introduced by the Gateway lifecycle changes.
hireview001
left a comment
There was a problem hiding this comment.
- CI failed: CI result (failure).
- CI failed: harness-validation / Acceptance (oceanbase) (failure).
- CI failed: harness-validation / Acceptance (sqlite) (failure).
- CI failed: opendal-package (failure).
- CI failed: Main (failure).
|
Thanks for investigating and checking the baseline. I also verified that:
|
|
Thanks for confirming, @MaoMengww. The head is still Please sync this branch with master and rerun the required CI checks. A local merge check is clean, but the PR still reports the failed pre-fix run, so successful CI recovery has not yet been demonstrated on this PR. |
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI failed: CI result (failure).
- CI failed: harness-validation / Acceptance (oceanbase) (failure).
- CI failed: harness-validation / Acceptance (sqlite) (failure).
- CI failed: opendal-package (failure).
- CI failed: Main (failure).
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
Flush the one-shot doctor JSON before exiting so failed loopback probes cannot keep the Node process alive past the CLI probe timeout.
Add a bundled product qualification for the a211ba8 merged Gateway deadline contract. The evidence covers all thirteen Desktop operations against the clean Server wheel and records the real SQLite harness results for anonymous, Bearer, explicit-CA HTTPS, and enforced Provider modes.
|
CI readiness update for
Base Please refresh the affected generated bundles, or incorporate the corresponding baseline fix, and rerun the checks before merging. Other current-head checks are still pending. |
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI failed: CI result (failure).
- CI failed: cindy-package (failure).
- CI failed: dsh-package (failure).
- CI failed: Main (failure). Failed steps: dsh-package / Test DSH package; cindy-package / Build and test the standalone Cindy archive; CI result / Require successful selected checks.
Merge upstream master at 195be9b to incorporate the Cindy and DSH pet bundle freshness fixes and the Linux Desktop clippy fix. Preserve both Gateway and Dashboard qualification histories and their original source, contract, artifact and evidence identities. Retain the historical authenticated-client entries and the existing qualification for the current Desktop contract.
| libc::_exit(1); | ||
| } | ||
| } | ||
| std::thread::sleep(Duration::from_millis(1)); |
There was a problem hiding this comment.
[P2] 退出看门狗与 drain 计时器都是 1 ms 轮询,代价随进程存活时间和连接数线性增长
gateway-exit-watchdog 从 install() 返回起就以 sleep(1ms) 循环。首个信号到来前 watchdog.started() 恒为 None,is_some_and 恒假,因此这个循环在整个进程生命周期内恒定约 950–1000 次/秒唤醒(按 1 天计约 8.6e7 次),每次唤醒做一次原子读加一次 Instant::now()。网关是常驻服务,这部分开销与请求量无关,是本次新增的常驻成本。
同一文件的 Deadlines::expired(第 75 行 deadline.min(now + Duration::from_millis(1)))在 source.is_some() 时也以 1 ms 为上限轮询,而 connection::drive 的 select 里每个活跃连接各自跑一份 drain_expired()。drain 窗口最长 25 s,于是 N 个并发模型连接会产生 1000×N 次/秒 的定时器唤醒(例如 100 个连接约 1e5 次/秒,持续 25 s),同时反复重排 tokio 定时器。
缩短后的 drain/exit 期限来自 started() 的 fetch_min(第 122 行),需求只是「更早的发布者写入后能被观察到」,并不需要 1 ms 精度:把轮询上限放宽到 50 ms,在 25 s 预算上只引入 ≤50 ms 的收敛误差。建议:未武装时让看门狗阻塞在 recv_timeout 或 Condvar 上(由运行时的普通任务在观察到 first_signal 后唤醒,信号处理器仍然只做原子写),武装后直接 sleep(start + EXIT - now);expired() 复用同一通知,或至少把轮询上限提到 50 ms 量级。
There was a problem hiding this comment.
Thanks, confirmed and addressed in 56f0c93.
The watchdog, shutdown signal waiter, and dynamic drain/exit timers now use a 50 ms observation interval. Armed waits are also capped by the remaining absolute deadline.
Local measurements showed idle watchdog wakeups dropping from roughly 920/s to 19/s. A single dynamic drain wait dropped from 25,002 polls to 502 over 25 seconds of virtual time.
The watchdog remains independent of Tokio. Requiring a Tokio task to wake or arm it would leave the non-yielding-runtime case unprotected. The real-process tests for stalled request execution and stalled runtime destruction still pass.
| /// 探活发现不了这种半关闭。因此池中借出的连接若在收到任何响应前 | ||
| /// 失败(broken pipe / EOF),丢弃该连接并用新连接重试一次。 | ||
| /// One operation budget covers pool acquisition, both attempts and complete framing. | ||
| /// Only read-only operations and exact capture bytes may replay after handoff. |
There was a problem hiding this comment.
[P2] prepare 的恢复链没有测试覆盖,且 acquire() 的注释与当前重试条件已不一致
request 现在只在 replay_safe || !handed_off 时重试;prepare 传入 replay_safe = false(第 432 行),所以它唯一的恢复路径是「写失败且已写字节为 0」。上一版 retryable() 对 closed before headers 有一条不区分操作的专门分支,本次收紧到按已写字节数判定,这个收紧本身是对的,但 acquire() 第 193–194 行的注释仍写着「真正的失效(对端已关闭)会在读写时报错,由上层的单次重试兜底」——它描述的正是池中那条被 uvicorn 关闭的空闲 keep-alive 连接,而现在这条兜底对 prepare 只在零字节移交时成立,注释读起来会让人以为它对所有操作无条件成立。
更实际的问题是这条恢复链在测试里完全没有覆盖:prepare_handoff_rejections_and_malformed_json_are_never_replayed 与 prepare_response_loss_is_not_replayed_after_handoff 都是失败链,readonly_and_exact_capture_recover_from_response_loss_over_http_and_uds 只覆盖 check 与 capture。也就是说「零字节写失败 → 新建连接 → prepare 成功」这条每次 keep-alive 空闲后都要走的正常路径没有用例。它依赖一个未在测试中固定的底层行为:AF_UNIX 对端 close() 后首写返回 EPIPE。若该行为变化(例如内核先缓冲整个小请求,让 written != 0),prepare 会在每次空闲 >keep-alive 超时后直接失败,而全部现有用例仍会通过。
建议补一条用例:服务端 accept 后立即关闭连接并把连接归还到池,随后 prepare 必须新建连接并成功(断言 accept 次数为 2);同时把 acquire() 的注释改成说明该兜底只对 replay_safe 操作与零字节移交成立。
There was a problem hiding this comment.
Thanks, confirmed the coverage gap and clarified the comment in 33eebdc.
The new regression test uses real Unix sockets and the public UdsGateClient. It first completes a reusable response, waits until the connection has returned to the pool, closes the server side, and then verifies that prepare reconnects successfully with exactly two accepted connections.
The acquire() comment now states the retry boundary explicitly: a retry requires either a replay-safe operation or proof that zero request bytes were handed off. Prepare remains non-replayable after handoff.
The existing response-loss tests still pass. I also verified prepare against a spawned real Python Core after a six-second idle period.
The watchdog, signal waiter and connection drain timers woke every millisecond even when no new shutdown state existed. Observe signal timestamps at 50 ms intervals while keeping the native watchdog independent of Tokio and capping armed sleeps at the absolute exit deadline.
Exercise the missing zero-byte recovery path through a real pooled Unix connection: Core closes an idle lease, prepare reconnects once, and the operation succeeds with exactly two accepted connections. Clarify that prepare cannot replay after any request bytes are handed off.
Which issue or RFC does this PR close?
Closes #2462
Engineering plan: Discussion #2433
Relevant contracts:
Rationale for this change
Gateway's existing transport and Core RPC timeouts do not bound an entire request. Slow request bodies, continuously trickling responses, multiple RPCs, retries, and pending response writes can each consume separate waiting windows. Client disconnects and service shutdown also need to propagate through the actual connection and resource owners.
This PR carries one request deadline and cancellation signal through ingress, the Executor, Core RPCs, model sending, and local HTTP/1 delivery while preserving model-send facts. Stopping local work does not establish provider rollback or allow an automatic resend of a possibly sent request.
What changes are included in this PR?
Are there any user-facing changes?
Connection: close. Fallback retains its independent concurrency budget, normal keep-alive, and streaming behavior, and remains subject to service shutdown deadlines.BoundedRetryandRetryOutcomeexports. Callers must route model retries through the Executor and supply a one-shot request control.How was this change tested?
Gateway workspace: 243 passed, 0 failed. API contract tests: 68 passed. Lifecycle E2E with a real Gateway and Python Core: 7 passed. Coverage includes total request deadlines, zero sends when stopped before handoff, no resend after uncertain sending, fresh admission, Core RPC recovery, connection delivery and resource reclamation, default shutdown budgets, and watchdog-enforced process exit.
Gateway authentication acceptance passed 33 tests (1 skipped), Desktop native regressions passed 133 tests (1 ignored), and website regressions passed 36 tests. Real SQLite Server qualification passed anonymous loopback, Bearer, HTTPS with an explicit CA/base path, and Provider authentication modes using a wheel verified against merge commit
5fb2b2d1. All 750 packaged runtime files match that source commit.make checkpassed in a clean checkout containing these changes, including lock consistency, all pre-commit hooks, type checks, and integration manifest checks.AI usage statement
Codex (GPT-6.1-Sol) assisted with implementation, testing, and documentation.