Skip to content

[PR] fix(watchdog): terminate the serving process from fatal worker - #5450

Open
Maziar123 wants to merge 9 commits into
ranxianglei:masterfrom
Maziar123:fix/event-loop-watchdog-process-exit
Open

Maziar123 wants to merge 9 commits into
ranxianglei:masterfrom
Maziar123:fix/event-loop-watchdog-process-exit

Conversation

@Maziar123

Copy link
Copy Markdown
Contributor

(ai: Codex/gpt-6.1-sol)

Problem

Follow-up to #4762 / #4784: the fatal watchdog runs inside a Node Worker. Calling process.exit(1) there exits only that worker, leaving the blocked serving process alive and its listening port occupied. Asking the parent to run an exit callback also cannot work while its event loop is blocked.

Change

After the existing synchronous FATAL forensic append, fatal mode signals the shared OS process ID with SIGKILL. Report mode remains nonfatal. Thresholds, heartbeat/unref/stop behavior, plugin-host isolation, and the dead-stderr safeguard are unchanged. A worker-created forensic log requests mode 0600.

The existing five-minute threshold is an availability policy, not proof of a deadlock. Legitimate exceptionally long synchronous computation can cross it, and forced termination can lose unflushed state. This change implements the already-intended fatal policy; it does not claim to fix the underlying expensive computation or guarantee a launcher restart.

Verification

  • Isolated runtime comparison against the original implementation: both children write one FATAL record; the original child remains wedged and retains its listener until fixture cleanup, while the candidate exits by SIGKILL and releases its listener without fixture cleanup. Candidate-created forensic file is 0600 on Linux.
  • node --import ./tests/preload-storage-baseline.mjs --import tsx --test tests/event-loop-watchdog.test.ts: 7 passed.
  • npm exec -- tsc --noEmit --project tsconfig.json: passed.
  • npm run check:unused-exports: passed, with the 8 pre-existing waivers reported.
  • npm run check:test-import-graph: passed.

No production process or live session was signalled during verification. No kernel source or version is changed.

@ranxianglei ranxianglei changed the title fix(watchdog): terminate the serving process from fatal worker [PR] fix(watchdog): terminate the serving process from fatal worker Nov 1, 2026
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

查重: 不重复。 本 PR 是 #4762/#4784 的后续——#4784 引入了「fatal 模式由 worker 自杀」的路径,本 PR 修的是该路径的实际缺陷(worker 的 process.exit(1) 只终止 worker 自己,serving 进程继续 wedged 占着端口)。没有其它 open issue/PR 处理同一缺陷;最接近的两个线程机制不同且已各自结案:#5152/#5153(watchdog 在自己 event loop 上取陈旧心跳读数)、#5085/#5086(heartbeat-death 分支需连续确认)。

标签: bug + scope/other + p2(缺陷在 serving 进程生命周期面、不属特定客户端车道;触发条件 = 主循环同步阻塞 >5min,有手动 kill 绕行、无数据丢失)。

审核进行中: 正对 PR head 505d0b6a5 逐行核对(分支落后 master 14 个提交,均为 release/CI 变更、不触碰本 PR 三个文件,预计无冲突);CI 侧 static-gates / kernel-guard / lane-guard / docs-coverage 已绿,test shards 仍在跑。结论按八问体例随后给出。

@ranxianglei ranxianglei added bug Something isn't working p2 scope/other labels Nov 1, 2026
…d with the constant un-export

The fix commit un-exported WATCHDOG_HEARTBEAT_MS / WATCHDOG_SOFT_WARN_MS / WATCHDOG_HARD_TIMEOUT_MS and removed the only test asserting their values (5_000 / 30_000 / 300_000), leaving the production thresholds unpinned. Re-adds the exports and restores the 'defaults are deadlock-scale' test verbatim from the pre-fix tree. Verified: watchdog suite 8/8, unused-exports gate clean (same 8 pre-existing waivers), tsc --noEmit clean.
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

Review 结论:方向正确、机制成立,建议合并(等新 head CI 绿)——已直接向分支补一个小修 commit

方向审核(原问题):#4762/#4784 的诊断成立。fatal worker 里 process.exit(1) 只终止 worker 线程本身,真正卡死的 serving 进程活着且继续占着监听端口;事件循环被阻塞时父进程也收不到/跑不了任何「请你退出」的回调。改成 process.kill(process.pid, "SIGKILL") 是唯一可靠出口:worker 与主线程共享 OS pid,SIGKILL 由内核投递,不能被 JS shutdown handler 捕获或延迟(循环卡死时 SIGTERM/SIGINT 的 Node handler 根本不会执行)。取证 appendFileSync 在 kill 之前同步完成,顺序正确。

回归清点(逐项过既有行为):

  • report 模式仍非致命 ✅(既有 report-mode wedge 测试保留并通过);
  • heartbeat / unref / stop 行为不变 ✅(beatTimer.unref()、worker error/exit → unwatched 降级、stop() 幂等均未动);
  • plugin host 隔离 ✅:armEventLoopWatchdog 全仓唯一调用点在 src/server.ts:677(import 在 :106),位于 startServer();dist/agent/* 注入代码从不调用 startServer → 卡死的宿主进程永远不会被自己的 watchdog 杀;
  • dead-stderr safeguard(Host-launched proxy can wedge on Windows when the parent stops draining stderr (pipe backpressure unguarded) #5012)不变 ✅;
  • 三个常量改非 export:grep 确认全仓无其他文件引用,无引用断裂;
  • 阈值数值本身未动(5s / 30s / 5min)。

行为变更披露(旧值 → 新值):

  1. fatal 路径:worker 自退出(对主进程是 no-op)→ 整个 OS 进程被 SIGKILL。这正是本 PR 的目的;
  2. worker 创建的 forensic 日志文件:umask 默认(~0644)→ 请求 0600(仅文件首次创建时生效,已存在的日志不受影响);
  3. 无监督部署(裸 bili start、docker 未配 --restart):5 分钟卡死后进程死亡,而不是 wedged 占端口。dead > wedged——健康检查能发现死亡,wedged 则永远挂着;有 spawner 的车道(claude Hook-model clients (claude/zcode) have no mid-session self-heal when the proxy dies — ECONNREFUSED until next session #1821 / zcode 会话内 respawn watcher、launcher rotateStalePredecessor)会在释放后的端口上自动重启。模块头注释现在诚实写明「5 分钟硬限是可用性策略,不是死锁证明」:合法的超长同步计算可能越过它,强杀可能丢未落盘状态。该权衡也已写入 CHANGELOG。

测试有效性:新 exit-mode 测试有牙——本地 A/B 实测:换回旧实现(process.exit(1))同一测试立即失败(子进程 wedged 到 20s deadline),新 head 通过。门禁双向齐全(report 非致命 + 健康不触发),端口重绑断言无 TIME_WAIT 风险(无已建立连接)。

已直接补进分支的小修(独立 commit 99b228e4d):原 fix commit 把三个常量改为非 export 的同时删掉了唯一断言它们取值的测试("defaults are deadlock-scale"),导致生产阈值 5_000 / 30_000 / 300_000 完全失去钉扎。已补回 export 并把该测试从修复前树逐字恢复。验证:watchdog 套件 8/8、unused-exports gate clean(仍是原有 8 条 waiver)、npm run typecheck clean。

结构化八问:

  1. 是什么问题:fatal watchdog 跑在 worker 线程里,process.exit(1) 只杀掉自己;真正卡死的 serving 进程继续活着并占用端口,「杀死进程让 spawner 重启」的设计此前整体失效。
  2. 什么原因导致的:Node worker 的 process.exit 不影响父进程;而父进程事件循环被阻塞后无法响应任何跨线程的退出请求——只有对共享 OS pid 的内核级信号(SIGKILL)能打破这个局面。
  3. 影响范围与严重程度:仅当代理主事件循环连续阻塞超过 5 分钟时触发(真实 wedged,如同步大 IO / 死锁);修复前后果 = 服务永久不可用 + 端口被占,修复后 = 进程死亡、受监督车道自动重启。p2(有绕行:手动 kill;触发面窄)。
  4. 怎么修复的:fatal 判定后先同步完成取证日志写入,再 process.kill(process.pid, "SIGKILL");report 模式不变;worker 创建的日志文件请求 0600。
  5. 是否有回归问题:无。五项既有行为(report 非致命、健康不触发、spawn 失败降级、dead-stderr 防护、unref/stop)逐项核查通过;常量去 export 未断任何引用;A/B 复现证明新测试钉住了原缺陷;本次补修只恢复既有测试与 export,无新语义。
  6. 是否破坏缓存:不涉及——不触碰压缩/缓存/重试/游标逻辑。
  7. 是否改了设计方向:不是转向——是把模块头注释原本声明、实现却未兑现的「fatal = 杀整个进程」语义真正实现;同时注释诚实补充了可用性策略边界(长同步计算可能被误杀、可能丢未落盘状态)。
  8. 是否引入了新的配置:无。没有新增 env/config/CLI 面。

CI 状态:原 head 505d0b6 全部快速 guard 绿(kernel-guard / lane-guard / pi-subagents-guard / static-gates / unused-exports / test-import-graph 等);test shard 矩阵因新 push 被自动取消(正常 churn,非失败);新 head 99b228e 的 CI 运行中,全套判定以远程 CI 为准。

合并判定:建议合并(待 99b228e CI 绿)。按 §7.4 不满足 auto-merge:无专属 issue 可 Fixes #N(本 PR 是 #4762/#4784 的后续),且变更了可用性策略行为——合并按惯例留给人工。

@Maziar123

Copy link
Copy Markdown
Contributor Author

(ai: Codex/gpt-6.1-sol)

Thank you for the review and 99b228e4d. I retained your commit by merging it into the contributor branch and pushed normally; no published commit was rewritten.

The follow-up removes the POSIX-only child signal assertion: Node 22 on Windows reported exit code 1 and a null signal for the same process termination. The regression still requires bounded child termination, the synchronous FATAL record, and successful listener rebinding without fixture cleanup. Report mode and the restored policy-boundary test remain.

At head d5f2f3131640c32c896485365ce13a7a6817bae5:

  • Local watchdog suite: 8 passed, with TypeScript and the unused-export gate passing.
  • Remote check rollup: 47 completed checks — 35 successful, 12 skipped; no pending or failed check. Skipped integration lanes are not represented as exercised coverage.

No production process or live session was signalled during qualification. The availability-policy tradeoff and possible loss of unflushed state remain documented; this does not identify the cause of historical long-running computation or guarantee an unsupervised restart.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

第二轮 Review 结论(d5f2f31):通过,可合并(人工合并)

Delta 审计(99b228e → d5f2f31,共 2 文件):

  • tests/event-loop-watchdog.test.ts:恰好删一行——assert.equal(outcome.signal, "SIGKILL")。这个删除是对的:Windows 上 process.kill(pid, "SIGKILL") 走 TerminateProcess,Node 的 close 事件报 code=1 / signal=null,POSIX-only 断言会让 Windows 车道确定性红(§7.2 跨平台确定性)。
  • 牙没有丢:区分原缺陷的断言是 timedOut === false(旧实现下子进程 wedged 到 20s deadline)+ 同步 FATAL 取证行 + 释放端口的重绑成功。「让监听持有者活着」的任何变体(worker-only exit、fatal 模式静默失效、kill 打错目标)都会在这几项上失败——我第一轮已实测 A/B:换回旧实现同一测试在 timedOut 项失败。
  • CHANGELOG:一句同步改为「不把 Windows 的退出状态误当作 POSIX 信号」,与代码事实一致。
  • 我的 commit 99b228e4d 确认保留在历史中(merge 方式并入,无重写)。

结构化八问(本轮 delta = 删一条平台相关断言 + changelog 一句):

  1. 是什么问题:上一轮的 signal === "SIGKILL" 断言只在 POSIX 成立;Windows 上 SIGKILL 经 TerminateProcess 投递,退出状态是 code=1/signal=null,Windows 车道会确定性红。
  2. 什么原因导致的:Node 在 Windows 不为 TerminateProcess 合成 POSIX 信号——平台语义差异,不是修复逻辑的缺陷。
  3. 影响范围与严重程度:仅测试断言层(Windows CI 红),运行时零影响;属构建阻断级。
  4. 怎么修复的:去掉平台相关断言,保留契约级断言(有界终止 + FATAL 记录 + 端口重绑);changelog 措辞同步。
  5. 是否有回归问题:无。src/ 零改动;report 模式非致命、0600 文件权限(non-win32)、默认值钉扎(上轮恢复的测试)全部仍在,本地新 head 8/8 通过;对原缺陷类无覆盖空洞(逐项推演见上)。
  6. 是否破坏缓存:不涉及。
  7. 是否改了设计方向:无——测试从「以何种方式死」收敛到「是否有界地死并释放端口」,后者才是用户关心的行为契约。
  8. 是否引入了新的配置:无。

状态核对:

  • 本地(新 head d5f2f31):watchdog 套件 8/8 通过;
  • CI(镜像缓存):30 项全部完成——18 success + 12 skipped(gated e2e 车道,正常跳过),0 failed / 0 pending;你报的 47/35/12 是矩阵实例粒度计数,结论一致;
  • 分支落后 master 18 个提交,但 PR 触碰的三个文件与 master 零文本重叠,无冲突面。

判定:可合并。按 §7.4 不 auto-merge(无专属 issue 供 Fixes #N,且含可用性策略行为变更)——请人工合并: #5450

@Maziar123

Copy link
Copy Markdown
Contributor Author

(ai: Codex/gpt-6.1-sol)

Thank you for the review and 99b228e4d. I retained your commit by merging it into the contributor branch and pushed normally; no published commit was rewritten.

The follow-up removes the POSIX-only child signal assertion: Node 22 on Windows reported exit code 1 and a null signal for the same process termination. The regression still requires bounded child termination, the synchronous FATAL record, and successful listener rebinding without fixture cleanup. Report mode and the restored policy-boundary test remain.

At head d5f2f3131640c32c896485365ce13a7a6817bae5:

  • Local watchdog suite: 8 passed, with TypeScript and the unused-export gate passing.
  • Remote check rollup: 47 completed checks — 35 successful, 12 skipped; no pending or failed check. Skipped integration lanes are not represented as exercised coverage.

No production process or live session was signalled during qualification. The availability-policy tradeoff and possible loss of unflushed state remain documented; this does not identify the cause of historical long-running computation or guarantee an unsupervised restart.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

这条评论与上一条转发内容相同(同一 head d5f2f31),我已在上面一条回复里完成了对它的完整审核,不再重复展开。快速核对:分支 head 仍为 d5f2f31,PR 仍 open/mergeable,无新提交——结论维持不变:

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

审查结论:APPROVE(可合,人工合并)

独立读了当前 head 23a41910c 的完整 diff(相对 merge-base 净 3 个文件:src/event-loop-watchdog.ts、其测试、CHANGELOG),未照抄 issue 侧结论。相比上一轮审到的 d5f2f31,增量只有 d6af37f4f(取证时间戳改用宿主 logTimestamp)加一处测试 flake 加固。无 kernel/ 触碰,无小修需要顺手改。

结构化八问

  1. 是什么问题:看门狗 exit 模式原来在 worker 里 process.exit(1),只杀掉看门狗 worker 自己,真正持有监听端口、已经卡死的代理主进程还活着占着端口 —— spawner「释放端口再重拉」的承诺落空([bug] After event loop wedge, zombie still holds port 18789 — parent respawn loop spins on EADDRINUSE indefinitely; manu | [bug] 事件循环卡死后僵尸仍握 18789 端口,parent 重拉全部 EADDRINUSE 死循环,只能手工杀进程恢复(event loop wedge holds listen socket, respawn loop fails with EADDR… #4762 后续)。
  2. 什么原因:worker_threads 是线程,与主线程共享同一 OS pid;process.exit() 在 worker 内只终止该线程;而「请主线程退出」的消息/回调又得靠那个卡死的事件循环来处理 —— 两条路都走不通,只能绕过事件循环直接对 OS pid 发不可捕获的信号。
  3. 影响范围与严重程度:仅在主事件循环连续阻塞 ≥5 分钟(HARD_TIMEOUT)这一极端故障态才触发,触发前行为完全不变。属可用性缺陷修复:原实现在最需要兜底时恰好失效(端口不释放、无法自愈),所以严重但触发面极窄。
  4. 怎么修复:worker 直接 process.kill(process.pid, "SIGKILL") 打向共享 OS pid;SIGKILL 不会被 JS shutdown handler 拖延,且 FATAL 取证行在发信号前已同步 append 完成。report 模式、健康心跳、stderr 保护、stop/unref 一律不动。附带把取证时间戳从 UTC ISO(Z) 换成宿主 logTimestamp 本地墙钟,并把 [level][v= 分隔对齐成宿主日志的 [level] [v=(d6af37f4f)。
  5. 回归风险:逐项过既有行为——report 模式逐字节不变(测试钉住);健康心跳不误杀(测试钉住);非 TTY 且有文件腿时不碰 stderr(Host-launched proxy can wedge on Windows when the parent stops draining stderr (pipe backpressure unguarded) #5012)不变;stop/unref 不变。唯一实质行为变更=致命路径从「对主进程 no-op」变「整进程 SIGKILL」,这正是本 PR 目的,且已在 PR 正文 + CHANGELOG + 模块头三处披露旧值→新值+原因。取证日志的时间戳/分隔符变化纯属可观测性对齐,grep 确认没有任何消费方解析旧的 [level][v= 相邻格式,不影响逻辑;worker 建取证文件请求 0600 是安全收紧。没有 fail-fast 变吞错,5 分钟硬阈值也没动(无时序漂移)。
  6. 是否破坏缓存:不涉及(纯进程生命周期 + 日志行格式)。
  7. 是否改了设计方向:没转向——仍是「worker 旁路监测 + 阈值策略」,只是把「退出」从 worker 自身改成 OS 级信号。诚实边界保留并写进模块头:5 分钟是可用性策略而非死锁证明,极长的合法同步计算也可能误杀并丢失未刷新的状态。
  8. 是否引入新配置:无。阈值仍硬编码(符合模块设计纪律),未加 env/config。

测试有效性:exit-mode 隔离子进程测试是真回归——阻塞主循环→watchdog SIGKILL→断言「有界终止(timedOut=false)+ 同端口重绑成功」;若回退到旧的 worker-only process.exit,子进程永不退出→超时报死且 EADDRINUSE,测试必红;跨平台(Windows 只断言有界终止+端口释放,不把退出状态误当 POSIX 信号)。新增两条格式断言有牙(正向正则钉本地时间无 Z + [level] [v=;doesNotMatch /Z \[/ 反向钉 UTC 回归)。把固定 sleep(80) 换成 3s 截止轮询是收紧不是放宽(降 runner 负载下的假阴,真死路径仍在预算后失败)。99b228e4d 补回了常量去导出后被丢掉的默认值 pin,防静默漂移。

diff 整洁度:净 3 文件、每行都服务本 PR;CHANGELOG 净 +1 行(diff 里看到的其它一大片条目是 master 合并带入、合并回 master 时为 no-op,不算本 PR scope creep);无空行/重排噪音。代码整洁(⑤b):命名达意、注释讲 why、无死代码、零新依赖,全过。

CI:head 23a41910c,static-gates / mysql-contract 已绿,test 矩阵分片仍在跑,目前无任何 failure(镜像缓存约 60s 刷新一次)。全套以远程 CI 为准,合并前请确认最终全绿。

判定:APPROVE。按 §7.4 不走自动合并(含可用性策略行为变更 + 无独立 Fixes#N issue,系 #4762/#4784 后续)→ 人工合并。未触碰 kernel/,无需 🔶 决策。review-worker-r1 完成。

… (conflict: CHANGELOG.md — kept both sides' Unreleased entries)
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

Round-2 verdict: APPROVE — head c5cacead7.

What changed since my round-1 review (23a41910c): exactly one commit — c5cacead7, a merge of current master into the PR branch. Its only conflict was CHANGELOG.md, resolved by keeping both sides' [Unreleased] entries.

  • PR code is byte-identical to what I already reviewed. src/event-loop-watchdog.ts and tests/event-loop-watchdog.test.ts are unchanged between the two heads (empty diff), so every correctness / test-teeth / regression conclusion from round 1 carries over verbatim: the SIGKILL-the-whole-process fatal fix, the isolated-child exit-mode regression test, the host-timestamp alignment, and the 0o600 forensic file.
  • The merge adds no new PR behavior. It pulls in already-shipped, already-CI-green master work; none of it touches the two watchdog files, so it cannot regress this PR's mechanism. No leftover conflict markers anywhere in the tree; the watchdog changelog bullet appears exactly once.
  • No kernel/ change, no kernel-change: declaration → no human-authorization gate applies.

CI for this head: 0 failures. Fast gates green (docs-sync, gate, lock-sync, mysql-contract, static-gates ubuntu-24, version-guard). The full test matrix (11 shards) plus red-signal and windows static-gates show as cancelled in this snapshot rather than passed, so a green full-suite result for this head isn't captured yet — please confirm the test lane is green before merging (branch protection enforces it regardless).

Merge recommendation: human merge per §7.4 — this is an availability-policy behavior change (fatal mode now terminates the whole serving process instead of just its worker) and the PR has no dedicated Fixes #N, so it doesn't meet the auto-merge gate. No blocking findings this round and nothing mechanical to spot-fix (the lone nit — duplicated ### Changed/### Fixed headers at the top of [Unreleased] from the union merge — matches the file's existing repeated-header convention and would take regrouping multi-contributor entries to clean, so it stays below the fix bar).

8问(中文)

  1. 是什么问题:看门狗跑在与主进程同 pid 的独立 Worker 里;旧 fatal 用 process.exit(1) 只杀掉 Worker,被卡死的服务主进程仍占着端口,守护重启撞 EADDRINUSE 起不来([bug] After event loop wedge, zombie still holds port 18789 — parent respawn loop spins on EADDRINUSE indefinitely; manu | [bug] 事件循环卡死后僵尸仍握 18789 端口,parent 重拉全部 EADDRINUSE 死循环,只能手工杀进程恢复(event loop wedge holds listen socket, respawn loop fails with EADDR… #4762)。本轮相对上轮仅多了一个「并入 master」的 merge commit,PR 代码零改动。
  2. 什么原因导致的:根因是杀错对象——process.exit() 只作用于调用它的线程(Worker),而真正要终止的是共享 pid 的整个进程;SIGKILL 作用于 OS 级 pid 才能连主进程一起带走并释放端口。
  3. 影响范围与严重程度:触发条件=事件循环连续阻塞≥HARD_TIMEOUT_MS(5min)且 fatalMode==="exit"(唯一 arm 点 src/server.ts:679,plugin 宿主 dist/agent/* 不 arm)。旧值→新值:旧=fatal 仅退 Worker、主进程存活占端口(可用性缺陷+端口泄漏);新=fatal 杀整个服务进程并释放端口。属 p1 级可用性缺陷的修复方向。
  4. 怎么修复的:同步写完 FATAL 取证日志后 process.kill(process.pid,"SIGKILL")(不可捕获)终止整个 OS 进程;取证文件收紧为 {mode:0o600};时间戳改用与宿主 logger 一致的本地墙钟(logTimestamp);report 模式/心跳(unref)/stop/stderr-TTY 守卫(Host-launched proxy can wedge on Windows when the parent stops draining stderr (pipe backpressure unguarded) #5012)均不变。
  5. 是否有回归风险:逐项过既有行为——report 模式、心跳 unref、stop()、stderr TTY 守卫、arm 点唯一性、SOFT/HARD 阈值常量全部保持;唯一语义变更=fatal 杀伤范围从「单 Worker」扩到「整个 OS 进程」,即修复目的且已披露。无 fail-fast 变吞错、无默认值/时序漂移;本轮 merge 未触碰这两文件,不新增回归面。
  6. 是否破坏缓存:不涉及——只在进程终止路径,不碰缓存/重试/增量游标/去重键。
  7. 是否改了设计方向:不涉及架构转向,仍是「Worker 内检测+fatal 时终止宿主进程」,仅把终止手段从线程级换为进程级 SIGKILL。
  8. 是否引入新配置:不涉及——无新 env/config/CLI flag。

判定:APPROVE(人工合并)。代码与上轮已审状态逐字节一致,本轮仅干净并入 master,CI 0 失败;合并前请确认测试矩阵转绿。

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

Round-2 review of PR #5450 (head c5cacead7) is complete — the full structured 八问 review is the comment directly above. Bottom line here:

Verdict: APPROVE — human merge (§7.4, not auto-merge: availability-policy behavior change + no dedicated Fixes #N).

Verified this round (independent pass, not carried over):

  • PR code (src/event-loop-watchdog.ts + tests/event-loop-watchdog.test.ts) is byte-identical to the already-reviewed head 23a41910c (empty diff); the only new commit is a clean merge origin/master (sole conflict CHANGELOG.md, union-resolved, zero leftover conflict markers tree-wide, watchdog bullet appears once).
  • No kernel/ change and no kernel-change: declaration → no human-authorization gate applies.
  • CI (this head): 0 failures; fast gates green (docs-sync / gate / lock-sync / mysql-contract / static-gates ubuntu-24 / version-guard).

Remaining before merge: the full test matrix (11 shards) + red-signal + windows static-gates read as cancelled in the cached snapshot rather than passed, so a green full-suite result isn't captured yet — confirm the test lane is green before merging (branch protection enforces it regardless). No mechanical spot-fix needed this round.

判定:APPROVE(人工合并)。代码与上轮已审状态逐字节一致,本轮仅干净并入 master,CI 0 失败;合并前请确认测试矩阵转绿。review-worker-r1 完成。

ci-check added 2 commits November 12, 2026 05:56
… (conflict: CHANGELOG.md — kept both sides' Unreleased entries)
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

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

Round-3 verdict: APPROVE — human merge. Head eeda19059.

Since my round-2 review (c5cacead7), the branch gained exactly two merge origin/master sync commits (c4b73e304a clean auto-merge incl. #6132/#6150; eeda190599 with the recurring CHANGELOG union resolution) plus the ~2 weeks of master work they pull in. No new watchdog-authored code.

Independent re-verification on eeda19059:

CI for eeda19059: 0 failures. Fast gates green (docs-sync, gate×3, lock-sync, mysql-contract, static-gates ubuntu-24); the full test matrix (12 shards) is in-progress, 11 integration lanes skipped — no cancellations this round, but a green full-suite result isn't captured yet. Confirm the test lane is green before merging (branch protection enforces it regardless).

No mechanical spot-fix needed this round.

8问(中文) — 代码与第 1、2 轮已审状态逐字节一致,八项结论不变,各一句:

  1. 是什么问题:看门狗 fatal 模式原在 worker 里 process.exit(1) 只杀 worker,卡死的代理主进程继续占端口、守护重启撞 EADDRINUSE([bug] After event loop wedge, zombie still holds port 18789 — parent respawn loop spins on EADDRINUSE indefinitely; manu | [bug] 事件循环卡死后僵尸仍握 18789 端口,parent 重拉全部 EADDRINUSE 死循环,只能手工杀进程恢复(event loop wedge holds listen socket, respawn loop fails with EADDR… #4762 后续);本轮仅多两个并入 master 的 merge commit,代码零改动。
  2. 什么原因导致的:worker 与主线程共享同一 OS pid;process.exit() 只作用于调用它的线程,须对共享 pid 发不可捕获信号才能终止整个进程并释放端口。
  3. 影响范围与严重程度:仅主事件循环连续阻塞 ≥5min(HARD_TIMEOUT)且 fatalMode==="exit"(唯一 arm 点在 startServer(),plugin 宿主 dist/agent/* 从不调用)触发;旧值→新值=fatal 从「仅退 worker」变「整进程 SIGKILL」,属 p1 级可用性缺陷的修复方向。
  4. 怎么修复的:同步写完 FATAL 取证后 process.kill(process.pid,"SIGKILL");取证文件收紧 {mode:0o600};时间戳对齐宿主 logTimestamp;report/心跳(unref)/stop/stderr-TTY 守卫(Host-launched proxy can wedge on Windows when the parent stops draining stderr (pipe backpressure unguarded) #5012)不变。
  5. 是否有回归风险:既有行为逐项保持(report 非致命、健康不误杀、stderr 守卫、stop/unref、arm 唯一性、SOFT/HARD 阈值常量);唯一语义变更即修复目的且已在正文+CHANGELOG+模块头三处披露;本轮两个 master merge 未触碰这两个文件,不新增回归面。
  6. 是否破坏缓存:不涉及——纯进程生命周期+日志行格式,不碰缓存/重试/增量游标/去重键。
  7. 是否改了设计方向:不涉及架构转向,仍是「worker 监测+fatal 时终止宿主进程」,仅把手段从线程级换为进程级 SIGKILL。
  8. 是否引入新配置:不涉及——无新 env/config/CLI flag。

判定:APPROVE(人工合并)。代码与两轮已审状态逐字节一致,本轮仅干净并入 master,CI 0 失败、无取消;合并前请确认测试矩阵转绿。review-worker-r1 完成。

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/other

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants