Code Review Summary (中文)

我已经完成了对 PR #2937 的详细代码审查。以下是关键发现:

‼️ 核心逻辑问题 (High Priority)

1. server.id 自动生成可能导致任务恢复失败

位置: GraphManager.java:72-76

当 server.id 为空时会自动生成随机 UUID:

server = "server-" + UUID.randomUUID().toString().substring(0, 8);

问题:

• 服务重启后会生成新的 server.id,导致之前分配给该节点的任务无法恢复
• StandardTaskScheduler.restoreTasks() 中移除了 selfServer.equals(task.server()) 过滤,现在会恢复所有任务,这与分布式场景不兼容

建议: 如果彻底移除多节点调度,应同时从 HugeTask 中移除 server 字段;或者基于持久化标识生成稳定的 server.id

2. 异常类型选择不当

位置: GraphManager.java:1131

原代码抛出 NotSupportException 表示类型转换失败,新代码改为 NotFoundException,但实际上图可能存在只是类型不匹配。

建议: 保持使用 NotSupportException,或改为更明确的异常信息说明是类型不匹配问题。

3. 调度器类型判断逻辑不完善

位置: StandardHugeGraph.java:1177-1178

return StandardHugeGraph.this.isHstore() ? "distributed" : "local";

问题:

• 仅基于 backend 类型判断,未考虑多节点 RocksDB 集群场景(通过 PD 协调)
• GraphManager.usePD() 方法已可见,应优先使用它判断

建议: 改为 return graphManager.usePD() ? "distributed" : "local";

⚠️ 重要问题 (Medium Priority)

4. 任务删除逻辑错误

位置: DistributedTaskScheduler.java:355-366

Line 360 的注释 "Already in DELETING status" 放在了 return null 之后,永远无法到达。逻辑不清晰,应该明确区分「未完成的任务」和「已经是 DELETING 状态的任务」。

5. 状态更新失败后的任务状态不一致

位置: DistributedTaskScheduler.cancel()

当 updateStatus 返回 false 时(数据库更新失败),内存中的 task 对象状态未同步。应该重新读取最新状态或抛出异常。

6. 等待超时可能导致事务泄漏

位置: DistributedTaskScheduler.close():399-410

如果 cron 任务超时,后续的清理可能在 cron 任务仍持有事务时执行。schedulePeriod 可能很大,建议限制最大等待时间。

7. 测试用例修改掩盖了潜在问题

位置: TaskCoreTest.java:1290-1296

原本期望状态为 CANCELLING,现在放宽为 "CANCELLING 或 CANCELLED 都行"。这种不确定性可能导致竞态条件难以调试。

🧹 代码质量建议 (Low Priority)

8. 配置注入可能覆盖用户设置

位置: GraphManager.java:1105-1111

应该检查 pdPeers 是否有效再注入:

if (!config.containsKey("pd.peers")) {
    String pdPeers = this.conf.get(ServerOptions.PD_PEERS);
    if (StringUtils.isNotEmpty(pdPeers)) {
        config.addProperty("pd.peers", pdPeers);
    }
}

9. TODO 注释应该追踪到 Issue

将 TODO 关联到 GitHub Issue,方便追踪;删除注释掉的代码。

总结

这个 PR 的核心改动是将任务调度从多节点模式简化为单节点模式,但存在以下关键风险:

1. 向后兼容性: server.id 随机生成会导致重启后任务恢复失败
2. 调度器选择: 应该基于 PD 存在性而非 backend 类型判断
3. 状态一致性: 多处状态更新缺少失败后的同步处理
4. 测试覆盖: 测试断言被放宽,可能掩盖并发问题

建议在合并前重点关注第 1-3 个核心逻辑问题。