Fail fast when overlap wait times out - #20
Conversation
JunzheJoe
left a comment
There was a problem hiding this comment.
All three conversions (get_engine_states failure, state-poll timeout, free-mem timeout) are warn-and-proceed → raise, which matches the fail-fast-only operational policy and closes the F2 review finding about the 60s overlap wait being warn-only — a timeout here previously let _before_training proceed into a likely wake_up OOM with only a log line as evidence.
Two notes:
- Scope check (good): the nvidia-smi-unavailable branch (
min_free_gb is Nonecounting) is untouched — that fail-open is a separate item pending Tao's per-item verdict, so no overlap. - Sequencing: this collides textually with #19 (same warn blocks, #19 keeps them as warns while refactoring). Suggested order: #19 first, this rebases onto it — the raises land unchanged in the refactored function. LGTM.
|
Follow-up after learning (via #21's closure) that
So: rebase, keep hunks 1–2, drop hunk 3. Approval stands for that reduced scope. |
|
change seems good, but needs to be implemented for updated miles_pipeline, this version is behind |
Context
_wait_for_overlap_engines_offloadedis the safety gate before training resumes on GPUs that may have just been used by inference engines. If the engine state probe fails or the wait times out, continuing can surface later as a less-informative train wake-up OOM.Change
Validation
python -m py_compile rlix/pipeline/miles_pipeline.py