feat: add health check based on write status #11607

[lzf575]

This PR is for:

[lzf575]

@claude review 下代码使用中文

[claude]

Claude finished @lzf575's task in 2m 23s —— View job

---

a. 事件类型：PR 评论触发（GENERAL_COMMENT），触发词是 “@claude review 下代码使用中文”。 b. 这是代码评审请求，不是实现请求；需要用中文给出 review 反馈。 c. 关键信息：PR #11680，分支 feat-add-ingester-status -> main；变更文件 3 个：@server/common/module_shared.go、@server/ingester/ingester/org_handler.go、@server/ingester/pkg/ckwriter/ckwriter.go。 d. 主要任务：阅读 diff 和相关上下文；检查功能正确性、并发安全、可维护性；整理成中文反馈。 e. 计划：先同步任务清单；再读取 diff（基于 origin/main...HEAD）和相关文件上下文；最后更新评论给出评审结论。如有必要指出建议的修复方向。 f. 本次按代码审查处理，不改动代码。

任务清单

• 理解触发评论中的请求
• 阅读 PR 改动和相关文件
• 整理中文评审意见
• 更新 GitHub 评论给出结论

Review

我看完这 3 处改动后，结论是：思路可以，但当前健康状态判定有两个比较明显的问题，建议修正后再合入。

1. @server/ingester/pkg/ckwriter/ckwriter.go:103 的健康判断过于宽松，会掩盖部分写失败

CKWriterManager.IsHealthy() 里现在的逻辑是：

• 只要任意一个 queueCtx.counter.WriteSuccessCount > 0，就直接返回 true
• 即使别的 queue 已经连续写失败，也会被这个早返回掩盖掉

这和“基于写状态做健康检查”的直觉不太一致。举个例子：

• queue A 曾经成功写过一次
• queue B 因为某个表、某个分片或某个 endpoint 一直写失败

当前实现仍然会把整个 ingester 判成健康，这样 controller 侧就看不到真实异常了。

更稳妥的方式通常是按 writer / queue 全量检查：

• 至少不要因为“某一个 queue 成功过”就直接判整体健康
• 更合理的是看是否存在“已经观察到失败，且该失败尚未被后续成功恢复”的 queue / writer

Fix this →

2. @server/ingester/pkg/ckwriter/ckwriter.go:103 / @server/ingester/pkg/ckwriter/ckwriter.go:106 直接读取计数器，存在数据竞争风险

WriteSuccessCount 和 WriteFailedCount 是在 CKWriter.Write() 中被 worker goroutine 持续更新的，见：

• @server/ingester/pkg/ckwriter/ckwriter.go:726
• @server/ingester/pkg/ckwriter/ckwriter.go:736
• @server/ingester/pkg/ckwriter/ckwriter.go:738

而 IsHealthy() 中只是给 ckwriterManager 加锁，并没有对这些 counter 的读写做同步。也就是说：

• ckwriterManager.Lock() 只能保护 m.ckwriters
• 不能保护 queueCtx.counter.* 这些由其他 goroutine 并发修改的字段

这会导致健康检查读到竞态中的值；如果开 -race，这里大概率会被打出来。

建议至少把这些计数改成原子读写，或者把健康检查需要的状态收敛到一个专门同步的字段里，而不是直接读裸计数器。

Fix this →

其他说明

• @server/common/module_shared.go:170 的 CheckIngesterStatus() 目前仓库里还只看到定义，暂时没搜到调用点；如果后续 controller 会频繁轮询这个状态，建议顺手评估一下日志级别：现在初始化和异常都会直接打 Info/Error，频繁调用时可能会刷日志。
• @server/ingester/ingester/org_handler.go:70 里的 IsHealthy() 直接返回 true 本身没问题，等于把健康检查职责集中到真正写 CK 的 handler 上，这一点是合理的。

结论

建议先修复后再合入。 主要是：

1. 当前整体健康判断会被“任一 queue 曾成功写过”误判为健康；
2. 健康检查直接读取并发更新的计数器，存在数据竞争。