Skip to content

Commit 6261f44

Browse files
committed
Fix diagnostics stop lifecycle
1 parent 067efe6 commit 6261f44

6 files changed

Lines changed: 536 additions & 22 deletions

File tree

flagscale/runner/backend/backend_megatron.py

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,17 @@
1818
from omegaconf import DictConfig, OmegaConf
1919

2020
from flagscale.runner.backend.backend_base import BackendBase
21-
from flagscale.runner.heartbeat.config import prepare_heartbeat_launch_config
21+
from flagscale.runner.diagnostics import (
22+
active_run_id_cleanup_lines,
23+
active_run_id_file,
24+
active_run_id_setup_lines,
25+
diagnostic_command_body,
26+
read_active_run_id,
27+
)
28+
from flagscale.runner.heartbeat.config import (
29+
HeartbeatLaunchConfig,
30+
prepare_heartbeat_launch_config,
31+
)
2232
from flagscale.runner.runner_train import (
2333
_get_args_megatron,
2434
_update_config_train,
@@ -50,7 +60,29 @@ def _prepare(self):
5060
self._prepare_perf_monitor_config()
5161
self.user_args = _get_args_megatron(self.config)
5262
self.rdzv_id = datetime.now().strftime("%Y%m%d_%H%M%S.%f")
63+
self.diagnostics_run_id = self.rdzv_id
64+
logging_config = self.config.train.system.logging
65+
self.active_run_id_file = active_run_id_file(logging_config.pids_dir)
5366
self.heartbeat_config = prepare_heartbeat_launch_config(self.config, self.rdzv_id)
67+
action = str(self.config.get("action", "run")).lower()
68+
if action == "stop" and self.heartbeat_config.enabled:
69+
active_run_id = read_active_run_id(self.active_run_id_file)
70+
if active_run_id is not None:
71+
self.diagnostics_run_id = active_run_id
72+
self.heartbeat_config = prepare_heartbeat_launch_config(
73+
self.config, self.diagnostics_run_id
74+
)
75+
else:
76+
logger.warning(
77+
"Heartbeat stop cannot locate a valid active diagnostics run id at %s; "
78+
"training workers will still be stopped",
79+
self.active_run_id_file,
80+
)
81+
self.heartbeat_config = HeartbeatLaunchConfig(enabled=False)
82+
self.publish_active_run_id = self.heartbeat_config.enabled and action in {
83+
"run",
84+
"test",
85+
}
5486
self.user_envs = self.config.experiment.get("envs", {})
5587
self.user_script = self.config.experiment.task.entrypoint
5688
self.resources = parse_hostfile(self.config.experiment.runner.get("hostfile", None))
@@ -128,6 +160,13 @@ def generate_run_script(
128160
f.write(f"mkdir -p {system_config.straggler_log_dir}\n")
129161
if system_config.get("perf_log_dir", None):
130162
f.write(f"mkdir -p {system_config.perf_log_dir}\n")
163+
if self.publish_active_run_id:
164+
for line in active_run_id_setup_lines(
165+
self.active_run_id_file,
166+
self.diagnostics_run_id,
167+
node_rank,
168+
):
169+
f.write(f"{line}\n")
131170
f.write("\n")
132171
f.write(f"cd {pkg_dir}\n")
133172
f.write("\n")
@@ -161,7 +200,12 @@ def generate_run_script(
161200
)
162201
f.write("\n")
163202

164-
command_body = self.heartbeat_config.training_command_body(node_rank)
203+
command_body = diagnostic_command_body(
204+
node_rank,
205+
self.heartbeat_config,
206+
active_run_id_path=(self.active_run_id_file if self.publish_active_run_id else ""),
207+
active_run_id=(self.diagnostics_run_id if self.publish_active_run_id else ""),
208+
)
165209
if background:
166210
f.write(
167211
f'nohup bash -c "{command_body}" >> {host_output_file} 2>&1 & echo $! > {host_pid_file}\n'
@@ -197,15 +241,21 @@ def generate_stop_script(self, host, node_rank):
197241
after_stop = ""
198242
with open(host_stop_script_file, "w") as f:
199243
f.write("#!/bin/bash\n\n")
244+
for line in self.heartbeat_config.stop_shell_lines(node_rank):
245+
f.write(f"{line}\n")
200246
f.write("if [ -f " + host_pid_file + " ]; then\n")
201247
f.write(" pid=$(cat " + host_pid_file + ")\n")
202248
f.write(" pkill -P $pid\n")
203249
f.write("else\n")
204250
# TODO: This is a temporary fix. We need to find a better way to stop the job.
205251
f.write(" pkill -f 'torchrun'\n")
206252
f.write("fi\n")
207-
for line in self.heartbeat_config.stop_shell_lines(node_rank):
208-
f.write(f"{line}\n")
253+
if node_rank == 0 and self.diagnostics_run_id != self.rdzv_id:
254+
for line in active_run_id_cleanup_lines(
255+
self.active_run_id_file,
256+
self.diagnostics_run_id,
257+
):
258+
f.write(f"{line}\n")
209259
f.write(f"{after_stop}\n")
210260
f.flush()
211261
os.fsync(f.fileno())

flagscale/runner/diagnostics.py

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
# Copyright 2026 FlagOS Contributors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Shared lifecycle helpers for opt-in runner diagnostics."""
16+
17+
from __future__ import annotations
18+
19+
import math
20+
import os
21+
import re
22+
import shlex
23+
from typing import Protocol
24+
25+
ACTIVE_RUN_ID_FILENAME = "diagnostics.active_run_id"
26+
_RUN_ID_PATTERN = re.compile(r"^[A-Za-z0-9_.-]+$")
27+
28+
29+
class DiagnosticLaunchConfig(Protocol):
30+
enabled: bool
31+
32+
def command_exit_actions(self, node_rank: int) -> list[str]: ...
33+
34+
35+
def active_run_id_file(pids_dir: str) -> str:
36+
"""Return the stable file used to find the currently active diagnostic run."""
37+
38+
return os.path.join(str(pids_dir), ACTIVE_RUN_ID_FILENAME)
39+
40+
41+
def read_active_run_id(path: str) -> str | None:
42+
"""Read and validate the active diagnostic run id."""
43+
44+
try:
45+
with open(path, encoding="utf-8") as file_obj:
46+
run_id = file_obj.read().strip()
47+
except OSError:
48+
return None
49+
50+
if not _RUN_ID_PATTERN.fullmatch(run_id):
51+
return None
52+
return run_id
53+
54+
55+
def active_run_id_setup_lines(path: str, run_id: str, node_rank: int) -> list[str]:
56+
"""Atomically publish one run id from node zero when its run script starts."""
57+
58+
if node_rank != 0:
59+
return []
60+
if not _RUN_ID_PATTERN.fullmatch(run_id):
61+
raise ValueError(f"Invalid diagnostics run id: {run_id!r}")
62+
63+
qpath = shlex.quote(path)
64+
qdir = shlex.quote(os.path.dirname(path))
65+
qrun_id = shlex.quote(run_id)
66+
return [
67+
f"mkdir -p {qdir}",
68+
f"diagnostics_run_id_tmp={qpath}.$$.tmp",
69+
f"printf '%s\\n' {qrun_id} > \"$diagnostics_run_id_tmp\"",
70+
f'mv -f "$diagnostics_run_id_tmp" {qpath}',
71+
]
72+
73+
74+
def active_run_id_cleanup_action(path: str, run_id: str) -> str:
75+
"""Remove the active marker only when it still belongs to this run."""
76+
77+
qpath = shlex.quote(path)
78+
qrun_id = shlex.quote(run_id)
79+
return (
80+
f'if [ -f {qpath} ] && [ \\"\\$(cat {qpath})\\" = {qrun_id} ]; '
81+
f"then rm -f {qpath}; fi"
82+
)
83+
84+
85+
def active_run_id_cleanup_lines(path: str, run_id: str) -> list[str]:
86+
"""Return direct shell lines that clear matching active state during stop."""
87+
88+
qpath = shlex.quote(path)
89+
qrun_id = shlex.quote(run_id)
90+
return [
91+
f'if [ -f {qpath} ] && [ "$(cat {qpath})" = {qrun_id} ]; then',
92+
f" rm -f {qpath}",
93+
"fi",
94+
]
95+
96+
97+
def diagnostic_command_body(
98+
node_rank: int,
99+
*configs: DiagnosticLaunchConfig,
100+
active_run_id_path: str = "",
101+
active_run_id: str = "",
102+
) -> str:
103+
"""Write monitor completion markers and clear active state on normal exit."""
104+
105+
actions = [
106+
action
107+
for config in configs
108+
if config.enabled
109+
for action in config.command_exit_actions(node_rank)
110+
]
111+
if node_rank == 0 and active_run_id_path and active_run_id:
112+
actions.append(active_run_id_cleanup_action(active_run_id_path, active_run_id))
113+
if not actions:
114+
return "$cmd; sync"
115+
return f"$cmd; rc=\\$?; {'; '.join(dict.fromkeys(actions))}; sync; exit \\$rc"
116+
117+
118+
def stop_process_shell_lines(
119+
pid_file: str,
120+
process_marker: str,
121+
*,
122+
timeout_s: float = 5.0,
123+
poll_interval_s: float = 0.1,
124+
) -> list[str]:
125+
"""Safely terminate one recorded diagnostic process with a bounded wait."""
126+
127+
attempts = max(1, math.ceil(timeout_s / poll_interval_s))
128+
qpid_file = shlex.quote(pid_file)
129+
qmarker = shlex.quote(process_marker)
130+
return [
131+
f"if [ -f {qpid_file} ]; then",
132+
f' diagnostics_pid="$(cat {qpid_file})"',
133+
' if [[ "$diagnostics_pid" =~ ^[0-9]+$ ]] && '
134+
f'ps -p "$diagnostics_pid" -o args= 2>/dev/null | grep -F -- {qmarker} >/dev/null; then',
135+
' kill "$diagnostics_pid" 2>/dev/null || true',
136+
f" for ((diagnostics_wait=0; diagnostics_wait<{attempts}; diagnostics_wait++)); do",
137+
' kill -0 "$diagnostics_pid" 2>/dev/null || break',
138+
f" sleep {poll_interval_s:g}",
139+
" done",
140+
' if kill -0 "$diagnostics_pid" 2>/dev/null; then',
141+
' kill -KILL "$diagnostics_pid" 2>/dev/null || true',
142+
" fi",
143+
" fi",
144+
f" rm -f {qpid_file}",
145+
"fi",
146+
]
147+
148+
149+
def wait_for_file_removal_shell_lines(
150+
path: str,
151+
*,
152+
timeout_s: float = 10.0,
153+
poll_interval_s: float = 0.1,
154+
) -> list[str]:
155+
"""Wait for node zero to finish stopping a shared diagnostic process."""
156+
157+
attempts = max(1, math.ceil(timeout_s / poll_interval_s))
158+
qpath = shlex.quote(path)
159+
return [
160+
f"for ((diagnostics_wait=0; diagnostics_wait<{attempts}; diagnostics_wait++)); do",
161+
f" [ ! -f {qpath} ] && break",
162+
f" sleep {poll_interval_s:g}",
163+
"done",
164+
]

flagscale/runner/heartbeat/config.py

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@
2323

2424
from omegaconf import DictConfig, OmegaConf
2525

26+
from flagscale.runner.diagnostics import (
27+
stop_process_shell_lines,
28+
wait_for_file_removal_shell_lines,
29+
)
30+
2631

2732
def _positive_float(value: Any, name: str) -> float:
2833
try:
@@ -100,8 +105,15 @@ def hardware_health_log_file(self, node_rank: int) -> str:
100105

101106
def training_command_body(self, node_rank: int) -> str:
102107
"""Run training and notify the node-zero heartbeat monitor on exit."""
103-
if not self.enabled:
108+
exit_actions = self.command_exit_actions(node_rank)
109+
if not exit_actions:
104110
return "$cmd; sync"
111+
return "$cmd; rc=\\$?; " + "; ".join(exit_actions) + "; sync; exit \\$rc"
112+
113+
def command_exit_actions(self, node_rank: int) -> list[str]:
114+
"""Return cleanup actions for a shared diagnostic command wrapper."""
115+
if not self.enabled:
116+
return []
105117
exit_actions: list[str] = []
106118
if node_rank == 0:
107119
completion_file = shlex.quote(self.completion_file)
@@ -112,9 +124,7 @@ def training_command_body(self, node_rank: int) -> str:
112124
f"if [ -f {health_pid_file} ]; then "
113125
f'kill \\"\\$(cat {health_pid_file})\\" 2>/dev/null || true; fi'
114126
)
115-
if not exit_actions:
116-
return "$cmd; sync"
117-
return "$cmd; rc=\\$?; " + "; ".join(exit_actions) + "; sync; exit \\$rc"
127+
return exit_actions
118128

119129
def shell_setup_lines(self, node_rank: int) -> list[str]:
120130
if not self.enabled:
@@ -214,23 +224,22 @@ def stop_shell_lines(self, node_rank: int) -> list[str]:
214224
if not self.enabled:
215225
return []
216226
lines: list[str] = []
217-
if self.hardware_health_enabled:
218-
health_pid_file = shlex.quote(self.hardware_health_pid_file(node_rank))
227+
if node_rank == 0:
219228
lines.extend(
220-
[
221-
f"if [ -f {health_pid_file} ]; then",
222-
f' kill "$(cat {health_pid_file})" 2>/dev/null || true',
223-
"fi",
224-
]
229+
stop_process_shell_lines(
230+
self.monitor_pid_file,
231+
f"flagscale.runner.heartbeat.monitor --heartbeat-dir {self.heartbeat_dir}",
232+
)
225233
)
226-
if node_rank == 0:
227-
pid_file = shlex.quote(self.monitor_pid_file)
234+
else:
235+
lines.extend(wait_for_file_removal_shell_lines(self.monitor_pid_file))
236+
if self.hardware_health_enabled:
228237
lines.extend(
229-
[
230-
f"if [ -f {pid_file} ]; then",
231-
f' kill "$(cat {pid_file})" 2>/dev/null || true',
232-
"fi",
233-
]
238+
stop_process_shell_lines(
239+
self.hardware_health_pid_file(node_rank),
240+
"flagscale.runner.heartbeat.gpu_health "
241+
f"--output-file {self.hardware_health_file(node_rank)}",
242+
)
234243
)
235244
return lines
236245

tests/unit_tests/runner/heartbeat/test_config.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ def test_disabled_heartbeat_is_a_noop(tmp_path):
3333
resolved = prepare_heartbeat_launch_config(_config(tmp_path, {"enabled": False}), "run")
3434
assert resolved.enabled is False
3535
assert resolved.shell_setup_lines(0) == []
36+
assert resolved.training_command_body(0) == "$cmd; sync"
37+
assert resolved.command_exit_actions(0) == []
38+
assert resolved.stop_shell_lines(0) == []
3639

3740

3841
def test_cloud_runner_rejects_enabled_heartbeat(tmp_path):
@@ -107,6 +110,31 @@ def test_optional_hardware_health_starts_one_node_local_cpu_collector(tmp_path):
107110
assert '\\"\\$(cat ' in resolved.training_command_body(0)
108111

109112

113+
def test_stop_orders_global_monitor_before_node_local_health(tmp_path):
114+
config = _config(
115+
tmp_path,
116+
{
117+
"enabled": True,
118+
"hardware_health": {"enabled": True},
119+
},
120+
)
121+
resolved = prepare_heartbeat_launch_config(config, "run-123")
122+
123+
node_zero = "\n".join(resolved.stop_shell_lines(0))
124+
node_one = "\n".join(resolved.stop_shell_lines(1))
125+
126+
assert node_zero.index(resolved.monitor_pid_file) < node_zero.index(
127+
resolved.hardware_health_pid_file(0)
128+
)
129+
assert "flagscale.runner.heartbeat.monitor --heartbeat-dir" in node_zero
130+
assert "kill -KILL" in node_zero
131+
assert "kill -0" in node_zero
132+
assert "[ ! -f" in node_one
133+
assert node_one.index(resolved.monitor_pid_file) < node_one.index(
134+
resolved.hardware_health_pid_file(1)
135+
)
136+
137+
110138
def test_hardware_health_command_timeout_must_be_less_than_interval(tmp_path):
111139
config = _config(
112140
tmp_path,

0 commit comments

Comments
 (0)