Skip to content

Commit a93089a

Browse files
authored
fix(dash): launcher shows live serving instances; amd-smi detection off critical path (eai-8190) (#295)
* fix(dash): seed launcher front door with live serving instances The launcher front door built an empty AppState, so it always rendered the idle variant even when a model was actively serving. Read the managed-service registry (the same authority `rocm services` reads) once per hub-loop pass and seed the AppState's instances from it. Also treat `Ready` as serving everywhere the dashboard counts running instances (`is_serving()`), matching the `Running`+`Ready` treatment already used elsewhere (e.g. home.rs) -- a served model reports `Ready`, not `Running`, so the count previously undercounted actual serving models. Adds apps/rocm's direct dependency on rocm-dash-core (previously only a transitive dep) so the launcher can build `Instance`s from the registry's `DiscoveredService` records. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(dash-daemon): run amd-smi detection off the run loop's critical path Detecting amd-smi (`amd-smi version` plus the first `system_info()`) can take up to ~15s on real hardware. Running it inline before the run loop's first tick blocked managed-service discovery and the first snapshot broadcast behind it, so an already-running model did not surface in the dashboard until GPU detection finished -- a visible ~15-20s "0 models running" lag while `rocm services` already reported it live. Spawn detection in the background and adopt the result via a oneshot channel the moment it lands, without ever blocking the loop while it is in flight. The loop now starts ticking immediately, so serving instances appear within one discovery tick; GPU metrics fill in once detection completes. Adds a regression test asserting on ordering (the instance must surface in a snapshot whose gpu_system_info is still None) rather than wall-clock timing, since a pure "arrived within Ns" check would be flaky under subscriber starvation. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(dash): exercise off-critical-path detection and launcher front door Address review feedback on the amd-smi-off-critical-path change. - Gate the "amd-smi unavailable" warning behind gpu_init_done so a healthy host never flashes it during the detection window; settle as unavailable (and say so once) if the detection task ever ends without a result. - Add an amd_smi_skip_kfd_preflight test seam so the daemon regression test drives a fake amd-smi through the real detection path instead of short-circuiting on a GPU-less CI host (the /dev/kfd guard stays mandatory in production). The test now genuinely fails if detection moves back onto the critical path, and asserts the surfaced snapshot carries no premature "amd-smi unavailable" warning. - Add direct unit tests for launcher_serving_instances (ready record maps to a live Instance; unbound :0 record is dropped) and a behavioural launcher scenario driving bare rocm through a PTY to prove the front door shows "Serving <model>" rather than "Idle" for a live registry service. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>
1 parent 8d9355c commit a93089a

11 files changed

Lines changed: 492 additions & 28 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/rocm/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ rocm-deps = { path = "../../crates/rocm-deps" }
2525
# rocm-dash unified dashboard launch. The
2626
# telemetry daemon + ratatui-0.30 TUI are launched from the `dash` verb; tokio
2727
# drives the async daemon/TUI from the otherwise-sync `rocm` binary.
28-
# (rocm-dash-core is pulled in transitively by the two below — not a direct dep.)
28+
# rocm-dash-core is a direct dep so the launcher front door can build serving
29+
# `Instance`s from the managed-service registry.
30+
rocm-dash-core = { path = "../../crates/rocm-dash-core" }
2931
rocm-dash-daemon = { path = "../../crates/rocm-dash-daemon" }
3032
rocm-dash-tui = { path = "../../crates/rocm-dash-tui" }
3133
tokio = { workspace = true }

apps/rocm/src/dash.rs

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ pub fn runner_options(
6666
// amd-smi ships inside the managed runtime wheel's bin dir, not on PATH;
6767
// resolve the path so the GPU collector can find it.
6868
amd_smi_binary: Some(rocm_core::resolve_amd_smi_binary()),
69+
// Production always runs the real `/dev/kfd` pre-flight; only daemon
70+
// integration tests with a fake binary skip it.
71+
amd_smi_skip_kfd_preflight: false,
6972
}
7073
}
7174

@@ -358,7 +361,12 @@ pub fn run_launcher(chat_mock: bool) -> Result<()> {
358361
let config = RocmCliConfig::load(&paths).unwrap_or_default();
359362
let theme = config.dashboard.tui.theme;
360363
loop {
361-
match rocm_dash_tui::ui::launcher::run_launcher(&theme)? {
364+
// Re-read the managed-service registry on each pass so the front door
365+
// reflects models started (or stopped) during a prior flow. This is a
366+
// cheap status-only file read — no telemetry daemon and no network
367+
// readiness probes, so the front door stays instant.
368+
let serving = launcher_serving_instances(&paths);
369+
match rocm_dash_tui::ui::launcher::run_launcher(&theme, serving)? {
362370
None => return Ok(()),
363371
Some(choice) => match launcher_route(choice) {
364372
LauncherRoute::Focused(focus) => run_focused(focus)?,
@@ -369,6 +377,28 @@ pub fn run_launcher(chat_mock: bool) -> Result<()> {
369377
}
370378
}
371379

380+
/// Serving instances for the launcher front door, read from the managed-service
381+
/// registry (the same authority `rocm services` reads).
382+
///
383+
/// Deliberately cheap: a status-only registry read with no network readiness
384+
/// probes and no telemetry daemon, so the front door renders instantly.
385+
fn launcher_serving_instances(paths: &AppPaths) -> Vec<rocm_dash_core::metrics::Instance> {
386+
use rocm_dash_daemon::registry::{discover_managed_services, load_service_records};
387+
let records = load_service_records(&paths.services_dir());
388+
discover_managed_services(&records)
389+
.svcs
390+
.into_iter()
391+
.map(|svc| rocm_dash_core::metrics::Instance {
392+
container_id: svc.container_id,
393+
container_name: svc.container_name,
394+
model_name: svc.model_name,
395+
status: svc.status,
396+
port: svc.port,
397+
..Default::default()
398+
})
399+
.collect()
400+
}
401+
372402
/// Entry point for a focused launcher flow (Set up / Serve / Diagnose).
373403
///
374404
/// Opens the dashboard runtime hosting exactly the one overlay for `focus` — no
@@ -916,4 +946,77 @@ mod tests {
916946
first.preferred_engines.first()
917947
);
918948
}
949+
950+
/// The launcher front-door seam that regressed: a `ready` managed vLLM
951+
/// service recorded on disk must map into a live `Instance` (id, model,
952+
/// port, status) so bare `rocm` shows it instead of "Idle". Reads the same
953+
/// registry `rocm services` reads — status-only, no daemon, no network.
954+
#[test]
955+
fn launcher_serving_instances_maps_ready_registry_record() {
956+
let root = std::env::temp_dir().join(format!(
957+
"rocm-cli-launcher-serving-test-{}-{}",
958+
std::process::id(),
959+
rocm_core::unix_time_millis()
960+
));
961+
let p = AppPaths {
962+
config_dir: root.join("cfg"),
963+
data_dir: root.join("data"),
964+
cache_dir: root.join("cache"),
965+
};
966+
let services_dir = p.services_dir();
967+
std::fs::create_dir_all(&services_dir).unwrap();
968+
std::fs::write(
969+
services_dir.join("svc.json"),
970+
r#"{"service_id":"vllm-launcher","engine":"vllm",
971+
"model_ref":"meta-llama/Llama-3.1-8B","canonical_model_id":"m",
972+
"host":"127.0.0.1","port":11435,
973+
"endpoint_url":"http://127.0.0.1:11435/v1","mode":"managed",
974+
"status":"ready","created_at_unix_ms":1}"#,
975+
)
976+
.unwrap();
977+
978+
let instances = launcher_serving_instances(&p);
979+
980+
assert_eq!(instances.len(), 1, "the ready managed service must surface");
981+
let inst = &instances[0];
982+
assert_eq!(inst.container_id, "vllm-launcher");
983+
assert_eq!(inst.model_name, "meta-llama/Llama-3.1-8B");
984+
assert_eq!(inst.port, Some(11435));
985+
assert_eq!(inst.status, rocm_dash_core::metrics::InstanceStatus::Ready);
986+
987+
let _ = std::fs::remove_dir_all(&root);
988+
}
989+
990+
/// A non-scrapeable record (unbound `port:0`) must NOT surface as a live
991+
/// instance on the front door — the registry is the authority and `:0` is
992+
/// never a real serving endpoint.
993+
#[test]
994+
fn launcher_serving_instances_skips_unbound_record() {
995+
let root = std::env::temp_dir().join(format!(
996+
"rocm-cli-launcher-serving-skip-test-{}-{}",
997+
std::process::id(),
998+
rocm_core::unix_time_millis()
999+
));
1000+
let p = AppPaths {
1001+
config_dir: root.join("cfg"),
1002+
data_dir: root.join("data"),
1003+
cache_dir: root.join("cache"),
1004+
};
1005+
let services_dir = p.services_dir();
1006+
std::fs::create_dir_all(&services_dir).unwrap();
1007+
std::fs::write(
1008+
services_dir.join("svc.json"),
1009+
r#"{"service_id":"vllm-unbound","engine":"vllm","model_ref":"m",
1010+
"canonical_model_id":"m","host":"127.0.0.1","port":0,
1011+
"mode":"managed","status":"ready","created_at_unix_ms":1}"#,
1012+
)
1013+
.unwrap();
1014+
1015+
assert!(
1016+
launcher_serving_instances(&p).is_empty(),
1017+
"an unbound (:0) record must not surface as a live instance"
1018+
);
1019+
1020+
let _ = std::fs::remove_dir_all(&root);
1021+
}
9191022
}

crates/rocm-dash-collectors/src/amd_smi.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,32 @@ impl AmdSmiCollector {
4343
/// directory rather than on `PATH`, so callers resolve the path or command
4444
/// name (via `rocm_core::resolve_amd_smi_binary`) and pass it here.
4545
pub async fn detect_with_binary(binary: impl Into<OsString>) -> Option<Self> {
46-
if !kfd_accessible() {
46+
Self::detect_with_binary_inner(binary, false).await
47+
}
48+
49+
/// Like [`detect_with_binary`](Self::detect_with_binary) but skips the
50+
/// mandatory `/dev/kfd` pre-flight.
51+
///
52+
/// **Test-only.** The KFD pre-flight is a safety guard: against a *real*
53+
/// `amd-smi` on a host without a usable `/dev/kfd`, the process can block in
54+
/// uninterruptible kernel sleep (D-state) that no signal can escape. This
55+
/// entry point exists solely so daemon integration tests can point
56+
/// [`detect_with_binary`](Self::detect_with_binary) at a *fake* script (for
57+
/// which the hang cannot happen) and have it actually run on a GPU-less CI
58+
/// host, instead of short-circuiting to `None` and turning the test into a
59+
/// no-op. Never call it against a real binary in production.
60+
#[doc(hidden)]
61+
pub async fn detect_with_binary_skipping_kfd_preflight(
62+
binary: impl Into<OsString>,
63+
) -> Option<Self> {
64+
Self::detect_with_binary_inner(binary, true).await
65+
}
66+
67+
async fn detect_with_binary_inner(
68+
binary: impl Into<OsString>,
69+
skip_kfd_preflight: bool,
70+
) -> Option<Self> {
71+
if !skip_kfd_preflight && !kfd_accessible() {
4772
return None;
4873
}
4974
let me = Self {

crates/rocm-dash-daemon/src/runner.rs

Lines changed: 85 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ pub struct RunnerOptions {
7474
/// so the caller resolves it (via `rocm_core::resolve_amd_smi_binary`) and
7575
/// passes it here. `None` falls back to looking up `amd-smi` on `PATH`.
7676
pub amd_smi_binary: Option<OsString>,
77+
/// **Test-only.** Skip the mandatory `/dev/kfd` pre-flight in amd-smi
78+
/// detection so a *fake* `amd_smi_binary` is actually invoked on a GPU-less
79+
/// CI host instead of short-circuiting to "no GPU". Never set in
80+
/// production: the KFD guard prevents a *real* `amd-smi` from hanging in
81+
/// uninterruptible D-state. Only the daemon integration test that points
82+
/// `amd_smi_binary` at a deliberately-slow fake script flips this, so the
83+
/// off-critical-path detection behaviour is genuinely exercised.
84+
pub amd_smi_skip_kfd_preflight: bool,
7785
}
7886

7987
impl Default for RunnerOptions {
@@ -93,6 +101,7 @@ impl Default for RunnerOptions {
93101
persist_dir: None,
94102
services_dir: None,
95103
amd_smi_binary: None,
104+
amd_smi_skip_kfd_preflight: false,
96105
}
97106
}
98107
}
@@ -200,23 +209,36 @@ pub async fn run_loop(
200209
// stable between scrapes (mirrors how GPU power drives tokens_per_watt).
201210
let mut per_container_used: HashMap<String, u64> = HashMap::new();
202211

203-
let gpu = match opts.amd_smi_binary.clone() {
204-
Some(binary) => AmdSmiCollector::detect_with_binary(binary).await,
205-
None => AmdSmiCollector::detect().await,
206-
};
207-
let mut gpu_system_info: Option<GpuSystemInfo> = if let Some(g) = &gpu {
208-
let info = g.system_info().await;
209-
info!(
210-
gpus = info.physical_gpu_count,
211-
model = %info.gpu_model,
212-
rocm = info.rocm_version.as_deref().unwrap_or("?"),
213-
"amd-smi detected"
214-
);
215-
Some(info)
216-
} else {
217-
warn!("amd-smi not available (no /dev/kfd or `amd-smi version` failed); GPU disabled");
218-
None
219-
};
212+
// amd-smi GPU detection plus the first `system_info()` can take up to ~15s
213+
// on some hosts (subprocess spawn plus the per-call detect/run timeouts).
214+
// Doing it inline here blocked the run loop's very first tick — and thus
215+
// managed-service discovery and the first snapshot broadcast — behind it, so
216+
// an already-running model did not surface in the dashboard until GPU
217+
// detection finished (a visible ~15-20s "0 models running" lag while
218+
// `rocm services` already reported it). Run detection off the critical path:
219+
// the loop starts ticking immediately (surfacing serving instances within
220+
// one discovery tick) and GPU metrics fill in the moment detection lands.
221+
let amd_smi_binary = opts.amd_smi_binary.clone();
222+
let amd_smi_skip_kfd_preflight = opts.amd_smi_skip_kfd_preflight;
223+
let (gpu_init_tx, mut gpu_init_rx) =
224+
tokio::sync::oneshot::channel::<(Option<AmdSmiCollector>, Option<GpuSystemInfo>)>();
225+
tokio::spawn(async move {
226+
let gpu = match amd_smi_binary {
227+
Some(binary) if amd_smi_skip_kfd_preflight => {
228+
AmdSmiCollector::detect_with_binary_skipping_kfd_preflight(binary).await
229+
}
230+
Some(binary) => AmdSmiCollector::detect_with_binary(binary).await,
231+
None => AmdSmiCollector::detect().await,
232+
};
233+
let info = match &gpu {
234+
Some(g) => Some(g.system_info().await),
235+
None => None,
236+
};
237+
let _ = gpu_init_tx.send((gpu, info));
238+
});
239+
let mut gpu: Option<AmdSmiCollector> = None;
240+
let mut gpu_system_info: Option<GpuSystemInfo> = None;
241+
let mut gpu_init_done = false;
220242

221243
let mut tick_count: u64 = 0;
222244
let mut last_sysinfo_refresh: u64 = 0;
@@ -234,6 +256,45 @@ pub async fn run_loop(
234256
// every instance refreshed in this cycle serialises Fresh deterministically.
235257
let cycle_at = Utc::now();
236258

259+
// Adopt the background amd-smi detection result the moment it lands,
260+
// without ever blocking the loop while it is still in flight. Until then
261+
// `gpu` stays `None` (GPU panels read "unavailable") but discovery and
262+
// snapshots run every tick, so serving instances appear promptly.
263+
if !gpu_init_done {
264+
match gpu_init_rx.try_recv() {
265+
Ok((detected, info)) => {
266+
gpu_init_done = true;
267+
if let Some(i) = &info {
268+
info!(
269+
gpus = i.physical_gpu_count,
270+
model = %i.gpu_model,
271+
rocm = i.rocm_version.as_deref().unwrap_or("?"),
272+
"amd-smi detected"
273+
);
274+
} else {
275+
warn!(
276+
"amd-smi not available (no /dev/kfd or `amd-smi version` failed); GPU disabled"
277+
);
278+
}
279+
gpu = detected;
280+
gpu_system_info = info;
281+
last_sysinfo_refresh = tick_count;
282+
}
283+
// The detection task dropped its sender without ever sending.
284+
// It has no fallible await today, so this is latent rather than
285+
// a live path — but if it ever regressed, polling `Empty`
286+
// forever would hang `gpu_init_done` at false and suppress the
287+
// honest "unavailable" warning below. Settle as unavailable and
288+
// say so once, so the badge resolves instead of silently never
289+
// clearing.
290+
Err(tokio::sync::oneshot::error::TryRecvError::Closed) => {
291+
gpu_init_done = true;
292+
warn!("amd-smi detection task ended without a result; GPU disabled");
293+
}
294+
Err(tokio::sync::oneshot::error::TryRecvError::Empty) => {}
295+
}
296+
}
297+
237298
let mut warnings = Vec::new();
238299
let gpus = if let Some(g) = &gpu {
239300
match g.metrics().await {
@@ -243,9 +304,15 @@ pub async fn run_loop(
243304
Vec::new()
244305
}
245306
}
246-
} else {
307+
} else if gpu_init_done {
247308
warnings.push("amd-smi unavailable (no /dev/kfd or binary missing)".into());
248309
Vec::new()
310+
} else {
311+
// Detection is still in flight (spawned off the critical path), so
312+
// `gpu == None` is transient here. Stay silent instead of flashing a
313+
// false "unavailable" alarm on healthy hardware for the ~15s window
314+
// until the first detection result lands.
315+
Vec::new()
249316
};
250317

251318
if gpu.is_some()

0 commit comments

Comments
 (0)