Add watchdog for stalled Ray initialization - #2016
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a watchdog mechanism to terminate the driver process if ray.init() hangs or fails to make progress within a configurable timeout specified by the SKYRL_RAY_INIT_TIMEOUT_IN_S environment variable. The review feedback suggests several robustness improvements: wrapping faulthandler.dump_traceback in a try-except block to guarantee process termination even if traceback dumping fails, safely parsing the timeout environment variable to prevent crashes on invalid values, and handling NaN timeout values to avoid raising exceptions in the threading library.
| logger.error(f"ray.init() did not complete within {timeout_s:g}s; terminating the driver") | ||
| faulthandler.dump_traceback(file=sys.stderr, all_threads=True) | ||
| os._exit(1) |
There was a problem hiding this comment.
If sys.stderr is redirected or replaced with a custom stream that does not support a file descriptor (e.g., in certain application servers, container environments, or testing frameworks), faulthandler.dump_traceback will raise a RuntimeError or AttributeError.
If an exception is raised here, the watchdog thread will crash before executing os._exit(1), leaving the stalled process hanging indefinitely.
Wrap the traceback dump in a try...except block to ensure that the driver is guaranteed to terminate even if the traceback dump fails.
| logger.error(f"ray.init() did not complete within {timeout_s:g}s; terminating the driver") | |
| faulthandler.dump_traceback(file=sys.stderr, all_threads=True) | |
| os._exit(1) | |
| logger.error(f"ray.init() did not complete within {timeout_s:g}s; terminating the driver") | |
| try: | |
| faulthandler.dump_traceback(file=sys.stderr, all_threads=True) | |
| except Exception as e: | |
| logger.error(f"Failed to dump traceback: {e}") | |
| os._exit(1) |
| # log_to_driver=True allows training progress from skyrl_entrypoint to reach stdout. | ||
| # Infrastructure logs (vLLM, workers) are redirected to log file via os.dup2 in their init. | ||
| ray.init(runtime_env={"env_vars": env_vars}, log_to_driver=True) | ||
| ray_init_timeout_s = float(os.environ.get("SKYRL_RAY_INIT_TIMEOUT_IN_S", "0")) |
There was a problem hiding this comment.
Parsing the environment variable SKYRL_RAY_INIT_TIMEOUT_IN_S directly with float() without handling potential parsing errors can cause the driver to crash with a ValueError if the variable is set to an empty string or an invalid non-numeric value (which is common in templated deployments).
Safely parse the environment variable with a try...except ValueError block and default to 0.0 (disabled) with a warning log.
| ray_init_timeout_s = float(os.environ.get("SKYRL_RAY_INIT_TIMEOUT_IN_S", "0")) | |
| ray_init_timeout_env = os.environ.get("SKYRL_RAY_INIT_TIMEOUT_IN_S", "0") | |
| try: | |
| ray_init_timeout_s = float(ray_init_timeout_env) if ray_init_timeout_env.strip() else 0.0 | |
| except ValueError: | |
| logger.warning( | |
| f"Invalid SKYRL_RAY_INIT_TIMEOUT_IN_S value: {ray_init_timeout_env!r}. " | |
| "Disabling the watchdog (timeout=0)." | |
| ) | |
| ray_init_timeout_s = 0.0 |
| if timeout_s <= 0: | ||
| return completed |
There was a problem hiding this comment.
If timeout_s is NaN (e.g., if parsed from a malformed float representation), the condition timeout_s <= 0 will evaluate to False. Subsequently, calling completed.wait(timeout_s) with NaN will raise a ValueError in Python's threading library, crashing the watchdog thread.
Add a check for math.isnan(timeout_s) to safely handle this edge case.
| if timeout_s <= 0: | |
| return completed | |
| if timeout_s <= 0 or math.isnan(timeout_s): | |
| return completed |
Summary
Motivation
Trajectory TCLI cold starts XIDs 1042972 and 1042974 both blocked indefinitely while the driver CoreWorker registered with the local raylet. The API process stayed healthy and create_model remained pending until its caller timed out after 3600 seconds. Because ray.init is synchronous, timing out only the caller leaves the unusable engine alive.
Set SKYRL_RAY_INIT_TIMEOUT_IN_S to enable the watchdog. The default remains disabled so existing deployments do not change behavior.
Test plan