[Feature] Add CI for fault tolerance #1222
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def simulate_crash(self): | ||
| logger.warning(f"Simulating crash on engine {self.server_host}:{self.server_port}...") | ||
| kill_process_tree(self.process.pid) |
There was a problem hiding this comment.
The new simulate_crash unconditionally dereferences self.process.pid, but self.process is only created in _init_normal; in --rollout-external mode (or any path where no local server process is launched), this attribute is never set. If CI enables fault injection in an external-engine setup, the Ray task will raise AttributeError instead of simulating a crash, which defeats the test and can break the rollout manager. Consider checking self.args.rollout_external (or self.process is not None) before calling kill_process_tree.
Useful? React with 👍 / 👎.
2 participants
No description provided.