来自 #268 的架构级评审建议。不阻塞合入,仅供参考是否有更好的架构解法。
⚠️ [重要 · 错误处理] 压测 get 把服务端错误当 key 不存在 cmd/ban-bench/runner.go:303
问题根因:压测 get() 对任何非 OK 状态一律返回 fmt.Errorf("key not found")。这与 PR 在转发层修掉的缺陷是同一类——服务端故障(StatusOverloaded、StatusError)会被静默当成「查询结果为空」。如果压测目标是监测服务端是否健康,这个错误路径会让故障被埋没为正常未命中,压测统计的 error rate 反而偏乐观。
为什么低级解法不够:通用评审会建议加个 switch 区分状态即可。这是确凿的、有 diff 证据的错误逻辑(它与 PR 声称修复的缺陷是同一模式),更重要的是它处在 PR 自己定义的『压测必须能拦住回归』的校验回路里——若服务端在 benchmark 期间频繁返回非 OK 状态,压测会把它们全部记成成功 get,CI 假绿。
架构级方案:让压测解析并区分状态码:StatusNotFound 记未命中,其余(StatusError/StatusOverloaded)记错误。这符合 PR 在 cmd/ban-bench 校验之外一贯主张的『区分键不存在与服务端故障』原则。考虑到压测刻意保留裸往返不走 SDK,至少应把它升级为能识别状态。
代价/收益:代价:压测代码需补一个状态解析分支(10 行内)。收益:压测统计真正反映服务端健康,与 PR 已在转发层建立的错误区分原则一致。
cmd/ban-bench/runner.go:303问题根因:压测 get() 对任何非 OK 状态一律返回
fmt.Errorf("key not found")。这与 PR 在转发层修掉的缺陷是同一类——服务端故障(StatusOverloaded、StatusError)会被静默当成「查询结果为空」。如果压测目标是监测服务端是否健康,这个错误路径会让故障被埋没为正常未命中,压测统计的 error rate 反而偏乐观。为什么低级解法不够:通用评审会建议加个 switch 区分状态即可。这是确凿的、有 diff 证据的错误逻辑(它与 PR 声称修复的缺陷是同一模式),更重要的是它处在 PR 自己定义的『压测必须能拦住回归』的校验回路里——若服务端在 benchmark 期间频繁返回非 OK 状态,压测会把它们全部记成成功 get,CI 假绿。
架构级方案:让压测解析并区分状态码:StatusNotFound 记未命中,其余(StatusError/StatusOverloaded)记错误。这符合 PR 在 cmd/ban-bench 校验之外一贯主张的『区分键不存在与服务端故障』原则。考虑到压测刻意保留裸往返不走 SDK,至少应把它升级为能识别状态。
代价/收益:代价:压测代码需补一个状态解析分支(10 行内)。收益:压测统计真正反映服务端健康,与 PR 已在转发层建立的错误区分原则一致。