Skip to content

[PR] fix: release input-only request data before streaming - #4756

Open
z0d0dOr wants to merge 18 commits into
ranxianglei:masterfrom
z0d0dOr:2026-10-25_request-input-lifetime
Open

z0d0dOr wants to merge 18 commits into
ranxianglei:masterfrom
z0d0dOr:2026-10-25_request-input-lifetime

Conversation

@z0d0dOr

@z0d0dOr z0d0dOr commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #4753.

Large requests retained their network chunks, raw Buffer, and preparation closure throughout long responses. This clears readBody's owned listeners/chunks on settlement, captures the raw-wire hash and comparability verdict before input mutation, and releases the raw/parsed input, affinity message alias, and preparation closure once overflow retries finish. Plugin bookkeeping still commits after forwarding.

Accepted outbound bodies remain available for fake-completion retries and echo provenance. Session pins and locking retain their existing lifetime. This is independent of the storage cold-load changes in #4750.

Validation:

  • 36 focused tests pass across nine files: body reading/cancellation, raw-vs-precomputed bookkeeping, release while an accepted stream is still pending, overflow/refold errors, fake-completion retries, cache capture, and same-session concurrency.
  • Listener regressions fail on the old implementation with a five-byte request; the five release-boundary regressions also fail before the fix and pass afterward.
  • TypeScript, build, unused-export, test-import-graph and diff checks pass. Full suite is left to CI; live Codex four-phase E2E remains a pre-merge check.

No kernel, configuration, persistence-format, or version changes. No runtime deployment. This removes demonstrated unnecessary retention; it does not establish that every possible OOM path is eliminated, and prepared messages/outbound retry data still remain live as required.

@ranxianglei ranxianglei changed the title fix: release input-only request data before streaming [PR] fix: release input-only request data before streaming Oct 25, 2026
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

查重:不重复。 已扫全部 open PR(含 #4750 存储冷加载、#4723/#4751 等近期内存/存储类修复)——没有第二条 PR 触碰 readBody 输入生命周期 / handleRequest 原始 Buffer 释放这条失败域。#4750 是存储冷加载(不同机制,PR 描述也已声明独立),#3437/#3643 讨论的是更宽的活跃请求物化面,均未实现本 PR 的输入生命周期清理。

标签: bug scope/wiring p2 pr —— 请求管道输入数据不必要滞留属性能/内存缺陷,有绕行(不 OOM 前可继续用),非缓存泄露类故定 p2。

开始按 PR Review 规范审:先读 #4753 与 diff 全量、核对既有行为清单,再本地定向验证,结论按结构化八问给出。

@ranxianglei ranxianglei added bug Something isn't working p2 scope/wiring labels Oct 25, 2026
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

Review 结论:可合并

① 方向审核(对照 #4753):#4753 点名的三个滞留点——readBody 的 listener/chunk 引用、转发后传给插件记账的 raw Buffer、overflow-refold 回调持有的 prepare 闭包——全部在根因处(生命周期管理)修掉,不是在出口层遮症状。边界划得也对:accepted outbound body(fake-completion/blind retry、echo provenance 要用的)与 session pin 生命周期原样保留,与 #4750 storage cold-load 正交。方向正确。

② 回归审查(diff 触碰路径的既有行为逐项清点):

  1. readBody 三条结算路径:成功路径移除全部四个 listener 并清空 chunks(新测试钉死「只剩外部 listener」);超限路径仍 destroy + reject BodyTooLargeError(limit)、error.limit 仍带 knob 值,行为不变;body 中途断开是行为变更(有意):旧实现无 close 处理,promise 永久挂起(handler 泄漏);新实现 reject request closed before body completed,调用方统一答 400 + warn log。全部 readBody 调用点(admin.ts / incremental-lane.ts / plugin-render.ts / handle.ts 两处)各有 try/catch,无 unhandled rejection。属改进非回归,按规则披露旧→新。
  2. 插件 wire 记账(hash gate / revisionEpoch / side-request guard):preparePluginWireMetadata 在 unwrap/prepare 突变前对已解析对象算 verdict,测试钉了 12 种 wire 形状与旧的「事后 parse」路径逐字节等价,并 mock JSON.parse 抛错证明不再 re-parse;hash-gate 语义(相同重试不脏、reformat 字节变 hash 但 verdict 不变不脏、verdict 翻转 bump epoch、saved-hash gate)均有回归钉。
  3. 释放时机:relay.ts 中 overflowRefold 唯一调用点在释放点之前(已读上下文确认);handle.ts try/finally 覆盖 transport 失败早退;释放闭包幂等。被释放变量(bodyBuffer/parsed/affinityMessagesCache/runPreparedPipeline)逐项 grep 确认 forward 之后零读取——INCOMING dump 发生在 runPrepare 内(pre-forward),REQ dump 用 outbound wireBody(设计上保留),memProbe 读 prepared.body,均不受影响。无观测性回归。
  4. 记账入口:rememberPluginMessages 全仓唯一调用点改传 rawWireMetadata;pipeline 块以 protocol 真值为前置条件,不存在 protocol 缺失时记账被跳过的缺口。
  5. 高危模式逐项:无语义漂移、无 fail-fast 变吞错、无阈值/时序/输出格式漂移。

③ 代码审查:整洁达标——命名达意、注释讲为什么、无死代码/魔法数字/新依赖。runPreparedPipeline! 非空断言略别扭但有明确理由(释放机制要求它可空),可接受。comparableHistory 是浅遍历,协议请求每次 eager 计算的代价相对本就逐次执行的 wire→core 转换可忽略。

④ 测试有效性:不是假测试,我实际做了 fail-on-old 验证——把三个新测试文件拷进 origin/master(bb7c249)worktree,全部按预期失败(readBody 取消测试因旧代码无 close handler 而挂起;新 API 缺失直接 import 失败);PR 分支上 16/16 绿;触碰改动函数的相邻套件(issue1307/2029/3400/plugin-fork/session-transient-memory)83/83 绿;typecheck 干净。断言有牙:trace 数组精确序列、release 恰好一次、stream-controller 在释放边界仍 pending 再完成的边界断言。

⑤ diff 整洁度:7 文件 +423/-26,每一行都服务本 PR 目的,无跑题改动、无锁文件/生成物噪音。

CI:head 9c8c72c 最新 run 全绿(含 real codex e2e 与全部 e2e 车道)。过程中有一次红 run(storage-render golden 的 metadata 段漂移 ×4 wire),签名与 #4599 已列档的既有 born-red 集群同族,同 SHA 重跑即绿,且本 PR diff 不触碰那些路径——判为继承性/flake,非本 PR 引入。

八问

  1. 是什么问题:大请求在长响应期间持续持有输入期数据(网络 chunk 引用、raw Buffer、解析后的 wire 对象、prepare 闭包),进程内存随请求尺寸长期驻留。
  2. 什么原因导致的:三处独立的生命周期缺陷——readBody concat 完成后不移除 listener/不清 chunks;raw Buffer 在转发后还被传进插件记账;refold 回调把整个 prepare 闭包带进上游重试期。都是「数据消费结束后无人接管所有权」,不是设计缺陷。
  3. 影响范围与严重程度:大请求 + 长响应的会话(agent 主循环正是此形状);程度为进程内内存持续增长、极端尺寸下有 OOM 风险;可用但有隐患,与所打 p2 一致。
  4. 怎么修复的:统一生命周期管理——readBody 在任何结算路径(含客户端断开)清掉自有 listener/chunks;wire hash + comparability verdict 在突变前算好只留标量;上游重试结束后释放输入变量与闭包(try/finally 兜底早退路径)。accepted outbound body 与 session pin 不动。
  5. 是否有回归问题:无。既有行为清点结论见上 ②;唯一有意的行为变更是 body 中途断开从「永久挂起」变为「400 + warn log」,已披露。
  6. 是否破坏缓存:不涉及。wire hash gate / revisionEpoch / 快照基线逻辑未动,且有等价性测试钉住。
  7. 是否改了设计方向:没有。纯生命周期修复,架构/边界/数据流均未转向。
  8. 是否引入新配置:没有。

判定:ready-to-merge。 跨 4 个 src 模块,按 §7.4 属人审层,请人工合并。

z0d0dOr and others added 8 commits October 25, 2026 12:35
The engineLane=server metadata added by f590da9 explains the entire drift: removing that one field reproduces the old metadata digest. Regeneration changes only metadata and the enclosing digest; all wire bodies, state, originals and statistics remain identical.
Compare unified reads with an independent envelope reader plus the documented migration. Persist alignment before rollback/corrupt-index checks, select legacy explicitly, and assert the recovered raw ID under its ledger ref. Keep checkpoint writes deterministic so sampled ingest cadence cannot change the seed between reader legs. All 28 targeted storage tests pass; no product migration or digest assertion is disabled.
Preserve both plugin compression routing and request wire metadata imports.
Keep the request input release and readBody listener cleanup behavior intact.

Reconcile storage tests with upstream's independent legacy oracle and
citation-normalized migration checks. Retain pre-migration exact digests,
rollback/corruption persistence, orphan restamp assertions, both fork storage
modes, Windows store cleanup, and exact launcher argument checks.

Validation: 28 request/metadata/readBody/concurrency/blog tests, 63 unchanged
fork/orphan/launcher tests, 17 final golden/storage proof tests, TypeScript
typecheck, and production build passed. Only targeted local suites were run.
Windows SIGTERM terminates the child instead of running its graceful
shutdown handler. Trigger the existing loopback shutdown route on Windows
so the draining-marker regression exercises the same lifecycle as POSIX.

Wait for child close before deleting its database, destroy sockets owned
by the silent TCP fixture, and stop health polling when the child exits.
Keep both regression scenarios and their behavioral assertions intact.

Validation: the isolated CI-preloaded serial test file passes 6/6 and exits
naturally in 11.58 seconds; npm run typecheck and npm run build pass.
Windows CI can reset an aborted probe connection instead of ending it.
The silent TCP fixture did not handle errors on accepted sockets, so
ECONNRESET became an uncaught test exception after the probe finished.

Destroy reset sockets and add a real resetAndDestroy regression against
the same fixture. Keep the existing port-owner and ladder assertions.

Validation: the new regression reproduces ECONNRESET before the fix;
the isolated CI-preloaded serial test file then passes 7/7 and exits
naturally. Pure tsc and git diff --check pass.
…eam conflict)

Single content conflict in tests/issue4982-draining-twin-ladder.test.ts resolved
by taking master's cross-platform POST /__bili/shutdown block (test#5050) while
keeping this branch's fixture improvements (killChild close-event+SIGKILL timer,
{close()} silent-TCP fixture, aborted-health-probe test) - union of both sides.
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] (ai: vllm-qwen/qwen3.8-27b)

上游冲突已解决,PR head 已更新。 把最新 master 合入 PR 分支(fast-forward、非强推),唯一一处内容冲突按「双方并集」处理,已推送到作者分支 z0d0dOr/billion-context:2026-10-25_request-input-lifetime(tip 9b99e4d52 → 4bbd5cb98)。

结构化八问

  1. 是什么问题:PR [PR] fix: release input-only request data before streaming #4756 因 master 前进而出现合并冲突(dirty),阻塞其合入 master。
  2. 什么原因导致:master 新增 14 个提交(release v0.1.209、[PR] release v0.1.209 #4982、fork-ledger [PR] fix: initialize fork ledgers atomically before publication #5044、CI fast-publish [PR] ci: 恢复紧急快速发布入口(fast 模式加双信号防误触门) #5054、test#5050 等),其中 test(windows-latest CI will always be red: the drain e2e in #4982 uses POSIX-only kill(SIGTERM) to trigger graceful shutdow | windows-latest CI 必红:#4982 的 drain e2e 用 POSIX 专属的 kill(SIGTERM) 触发优雅退出,Node 在 Windows 上是直接硬杀 #5050)与本分支各自独立修改了同一个 tests/issue4982-draining-twin-ladder.test.ts 的同一处(P1 优雅关停触发块),形成唯一一处文本冲突;其余被双方触碰的同名核心文件(src/server.ts、src/server/handle.ts、src/server/relay.ts、tests/proxy-readbody.test.ts)均无文本重叠,自动合并。
  3. 影响范围与严重程度:仅阻塞本 PR 合并,不影响任何已发布行为;属流程性阻塞,非功能缺陷。
  4. 怎么修复:git merge --no-ff origin/master(双亲合并提交 4bbd5cb98)。唯一冲突取并集而非单边:关停块采用 master 更完善的跨平台 POST /__bili/shutdown(try/catch,注释引 windows-latest CI will always be red: the drain e2e in #4982 uses POSIX-only kill(SIGTERM) to trigger graceful shutdow | windows-latest CI 必红:#4982 的 drain e2e 用 POSIX 专属的 kill(SIGTERM) 触发优雅退出,Node 在 Windows 上是直接硬杀 #5050/安装新版本后旧代理永远不换新:detach 代理躲过应用退出,重启只是重新挂上旧进程(「永远更新不了」) #3544),同时保留本分支对该文件 fixture 的改进(killChild close-event + 10s SIGKILL timer、{close()} silent-TCP fixture、新增 aborted-health-probe 测试)——两侧都是有效的健壮性加固,取任一整文件都会回退另一方的工作。已核验:无残留冲突标记、master 块在位、本分支三处改进均在位。
  5. 是否有回归:未对 PR 自身逻辑做额外改动;src 侧出现的变更均为 master 既有(本身已绿)经合并带入,PR 的 input-lifetime 核心(readBody 生命周期 / release 边界)未被二次改写。高危模式逐项:无语义漂移、无 fail-fast 变吞错、无阈值/时序/输出格式漂移(测试内 20s→30s 等待是 master 侧既有的有意变更,随 master 一并吸收)。全量回归交由 CI 在推送后的 head 上判定。
  6. 是否破坏缓存:不涉及(纯测试文件 + 合并 master,无缓存/重试/增量游标/去重键逻辑变更)。
  7. 是否改了设计方向:不涉及。
  8. 是否引入新配置:不涉及。

一点提醒(供 review 留意,我未擅自改动):该 PR 分支当前在原单提交之外还累积了约 8 个先前 sync 会话的测试/合并提交(Windows 优雅关停、health-probe fixture、storage render-golden 等),超出了最初「input-lifetime」单提交的范围;它们已在远端,若要收敛需改写作者分支历史(强推),不在本次「解冲突」的职责之内,留给你判断是否需要单独整理。

收工判定:冲突已解、PR head 已指向 4bbd5cb98;待 CI 在该 head 上转绿后由你手动合并(Agent 不合并 PR)。

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)

结果确认:PR #4756 的上游合并冲突已解决。

  • 把最新 master 合入 PR 分支(双亲合并提交 4bbd5cb98);唯一一处冲突(tests/issue4982-draining-twin-ladder.test.ts)按「双方并集」处理——采用 master 的跨平台 POST /__bili/shutdown 块,同时保留本分支对该测试 fixture 的三处改进(killChild timer、{close()} silent-TCP fixture、aborted-health-probe 测试)。
  • fast-forward(非强推)推送到作者分支 z0d0dOr/billion-context:2026-10-25_request-input-lifetime,PR head 已指向 4bbd5cb98。
  • 已核验:PR mergeable=true,mergeable_state 由 dirty 转为 behind(master 又前进了几个提交,非冲突、可直接合并)。

完整八问分析见上一条回复。待 CI 在新 head 上转绿后由你手动合并(Agent 不合并 PR)。另提醒:该分支在原单提交之外累积了约 8 个先前 sync 会话的测试/合并提交,超出最初范围,是否收敛留你判断。

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b) Synced origin/master into 2026-10-25_request-input-lifetime (merge commit d031f3e, no conflicts); local typecheck + build clean and the PR's targeted tests (request-input-lifetime, proxy-readbody, plugin-wire-metadata) pass 16/16 — branch is no longer behind, merge state now blocked pending CI checks.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

CI 红了,但不是本 PR 的问题——是继承自 master 的 #5144「cold raw-tail fork」flaky test,我触发 rerun 后仍复现,任何 merge 都修不了。已按「非小修」路径升级处理,未改本 PR 分支一行代码。逐条回答:

  1. 是什么问题:三个 test shard(ubuntu 22/3、ubuntu 24/3、windows 22/4)在同一条子测试上挂——HTTP a cold-restored non-retained parent serves its intact raw tail immediately (#5140),断言 409 !== 200(文件 tests/plugin-fork.test.ts)。
  2. 什么原因导致的:这是 [PR] fix: recover non-retained parent fork snapshot from intact unified raw tail after cold restart (#5140) #5144「cold raw-tail fork」特性自带的 flaky test,在 CI matrix 并发负载下偶发返回 409(冲突)而非 200。根因在 master 侧那个特性的实现/测试,不在本 PR。已核实:三个失败 job 签名一致;master 今天自身也红且本 PR 的红 shard 是其子集;PRhead 之后没有 master commit 触碰该测试或 [PR] fix: recover non-retained parent fork snapshot from intact unified raw tail after cold restart (#5140) #5144;ci-fixes@cf99d935f ⊂ master@a8c907dd5 → merge ci-fixes 是 no-op。
  3. 影响范围与严重程度:影响面 = 恰好把该子测试排进这几个 shard 且负载够高的运行;本 PR 及其它就绪 PR 被 required checks 卡住无法合并。严重度 = 阻塞合并(门禁误伤),非数据丢失/功能损坏。
  4. 怎么处理的:定性为继承红 → 触发 rerun 并确认重跑仍红(排除一次性阵发)→ 把完整判定 + 选项 + 建议报到 CI Control Center: Centralized CI Failure Handling & Status Sync (Comment all CI issues here; do not open new issues) | CI 总控: CI 失败集中处理与状态同步(所有 CI 问题在此评论, 勿开新 issue) #4599 交哨兵/owner 处置(选项 A 哨兵 ci-fixes 加固[推荐] / B admin override 临时放行 / C 保持红等 master 修)。本 PR 侧零改动。
  5. 是否有回归问题:无——我没动任何代码;本 PR 自己的改动(request-input-lifetime:server.ts/handle.ts/relay.ts/plugin.ts)与此 flake 无关,不受影响。
  6. 是否破坏缓存:不涉及(纯 CI 门禁问题,无运行时行为变更)。
  7. 是否改了设计方向:不涉及。
  8. 是否引入新配置:不涉及。

为什么没开新的 decision-request issue / 为什么没在分支上修:CI 协调 owner policy(2026-10-24)明确「一切 CI 问题去 #4599、禁止为 CI 问题开新 issue」,且 #4599 已有同类决策以评论形式处置的先例;所以我把决策请求作为评论报到 #4599(含选项 + 推荐 + 移交哨兵),而不是另开 issue。修它需要 master/ci-fixes 侧工作,不属于本分支小修,故未 push。

收工判定:本 lane(attempt 2/2)到此为止——CI 红已定性为继承 flake 并移交 #4599 哨兵/owner;本 PR 无需改动,待 #4599 侧根因收敛(或 admin 放行)后即可绿。

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Merge-guard sync done: merged origin/master through cdca39b7 (~50 commits, all clean auto-merges, no conflicts) into 2026-10-25_request-input-lifetime, pushed head 9f5e8248 which fully contains live master — state now blocked (CI checks pending), no longer behind. Local: typecheck + build clean, targeted suites 98/98 + 37/37 green. Bonus: the previously-flagged inherited red (tests/issue4982-draining-twin-ladder.test.ts, StorageSecondWriterError) is now GREEN here — your #5236 decision (3a6ab628 second-writer no-block) fixed it; no action needed from this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working p2 scope/wiring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: release request input buffers before long-running responses

2 participants