已合并
[bugfix] NodeManager判断register成功逻辑有误 #543
[bugfix] NodeManager判断register成功逻辑有误 #543
已合并
liu创建于 7月17日
liu
liu成员
7月17日

1. 合入背景

实例重调度后,在实例启动时触发controller备升主。实例日志中打印注册成功,但controller日志显示中实例注册被拒绝并被标记为inactive
Fixes#333

2. 修改内容

描述修改内容的具体实现,涉及哪些组件之间进行交互,可以用1、2、3、...进行罗列。
如果是需求或者重构类的PR,需要补充详细设计文档(说明上下游组件关系、时序图、类图、DFX能力等内容)。

3. 资料变更

请确认是否涉及资料变更
如涉及,需要在PR中体现,并简要说明修改内容。
如不涉及,需填写“不涉及”。

4. 接口变更

请确认是否涉及跨代码仓或者客户面可见的接口变更
如涉及,需详细说明接口以及对应的变更内容,同时需要在资料中体现。
如不涉及,需填写“不涉及”。

5. 测试结果

需体现测试场景,测试方法以及测试结果
测试用例设计时需考虑硬件、部署方式、功能、性能、精度、显存等维度。

6. CheckList

PR提交人对以下CheckList自检项进行全量自检,自检通过或不涉及,均修改 [ ] 为 [x]

[ ] 代码注释完备

[ ] 正确记录维测日志

[ ] 是否有UT用例

[ ] 若涉及多线程场景,考虑了并发场景,不存在死锁问题

likedislike
Pull Request已成功合入, 合并人@wangyang
(感谢 liu 的贡献)
liuliu成员
7月17日 关联了issue:[bugfix] NodeManager判断register成功逻辑有误
atomgit-bot
atomgit-bot
7月17日 评论:

变更摘要

此 PR 修复了 NodeManager 中注册成功判断逻辑的缺陷:原先的 ControllerApiClient.register 方法仅以 HTTP 请求无异常作为注册成功的依据,未校验响应内容;而 register_after_restore 方法则有完整的响应类型和错误码检查。由于备升主场景下 heartbeat 恢复后实际调用的是 register_after_restore,导致更严谨的校验仅在该路径生效。此变更将两套注册逻辑合并统一,所有注册路径均采用带响应校验的实现,解决了 controller 在注册请求看似成功却将实例标记为 inactive 的问题。

主要改动

  • 合并注册方法:删除原有的 ControllerApiClient.register(仅检查 HTTP 异常)和 ControllerApiClient.register_after_restore,统一为新的 ControllerApiClient.register(包含响应 dict 类型检查与 error 字段校验),确保所有注册调用均经过严格响应验证。
  • 移除冗余转发方法:删除 EngineManager.post_register_msg_after_restore 方法,消除仅用于调用旧 register_after_restore 的中间层。
  • 统一调用入口HeartbeatManager._register_after_restore 中将对 post_register_msg_after_restore 的调用改为 EngineManager().post_register_msg(),使备份升主后的注册路径与其他场景保持一致。
likedislike
atomgit-bot
atomgit-bot
7月17日 评论:

代码审查

审查总结

已完成对全部 3 个变更文件的审查:

  • motor/node_manager/api_client/controller_api_client.py:将旧的 register(无响应验证)与 register_after_restore(有响应验证)合并为统一的 register 方法,修复了 #333 所述的「注册返回 success 但 controller 将其标记为 inactive」的 bug。合并逻辑正确,无新增问题。

  • motor/node_manager/core/engine_manager.py:删除了 post_register_msg_after_restore 方法,post_register_msg 现调用统一后的 ControllerApiClient.register(),两处注册入口(初始注册 _register 和恢复后注册 _register_after_restore)均获得响应验证能力。逻辑正确。

  • motor/node_manager/core/heartbeat_manager.py_register_after_restore 中调用从 post_register_msg_after_restore() 改为 post_register_msg(),与其他注册路径统一。逻辑正确。

发现统计

优先级 数量
P0 0
P1 0
P2 1
P3 0

整体风险评估:中等风险。核心逻辑修复正确且完整,但删除 post_register_msg_after_restore 方法后未同步更新测试文件 tests/node_manager/test_heartbeat_manager.py 中的 5 处引用,将导致 3 个测试用例失败、CI 阻塞。建议在合入前修复测试。

类型 数量
🔴 阻塞 0
🟡 建议 1

💬 仅评论

likedislike
ascend-robotascend-robot成员
7月17日 添加了label:ascend-cla/yes
ascend-robot
ascend-robot成员
7月17日 评论:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here
You can get sig-info at here


PR Approval Progress

Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
repo-Ascend/MindIE-PyMotor codeDogPro, ganglv (2/2) codeDogPro (1/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

lbr711, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
此处折叠了71条消息 查看更多
ascend-robotascend-robot成员
7月20日 添加了label:approved
ganglv成员
7月20日 评论:

/lgtm

likedislike
ascend-robotascend-robot成员
7月20日 添加了label:lgtm
wangyangwangyang成员
7月20日 关闭了关联的issue
wangyangwangyang成员
7月20日 合入了pull request