fix(dash): launcher shows live serving instances; amd-smi detection off critical path (eai-8190) - #295
Open
r0x0r wants to merge 2 commits into
Open
fix(dash): launcher shows live serving instances; amd-smi detection off critical path (eai-8190)#295r0x0r wants to merge 2 commits into
r0x0r wants to merge 2 commits into
Conversation
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>
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>
r0x0r
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes so the dashboard's model/instance count reflects reality promptly:
run_launcherbuilt an emptyAppState; it now reads the managed-service registry (the same authorityrocm servicesreads) once per hub-loop pass and seeds the front door's instances from it. Also normalizes "serving" everywhere in the dash UI tois_serving()(Ready+Running), since a served model reportsReady, notRunning— the running-instance counts previously undercounted.amd-smidetection (amd-smi version+ firstsystem_info()) ran inline before the telemetry run loop's first tick, blocking managed-service discovery and the first snapshot broadcast behind it. Detection is now spawned in the background and adopted via aoneshotchannel the moment it lands, so serving instances surface within one discovery tick instead of waiting on GPU detection.Commits
fix(dash): seed launcher front door with live serving instancesfix(dash-daemon): run amd-smi detection off the run loop's critical pathTesting
cargo test -p rocm-dash-daemon -p rocm-dash-tui -p rocm --lib --tests— all passing, including two new unit regression tests (launcher_state_seeds_serving_from_registry_instances,serving_detail_counts_ready_instances_as_running) and one new daemon-level regression test (slow_gpu_detection_does_not_delay_service_discovery, which asserts on snapshot ordering rather than wall-clock timing to avoid flakiness under subscriber starvation).cargo clippy -p rocm -p rocm-dash-tui -p rocm-dash-daemon --all-targets -- -D warnings— clean.Cucumber e2e coverage gap: no existing scenario in
tests/e2e-cucumber/features/dash.featurecovers the bare-rocmlauncher front door or GPU-detection timing, and none were added here — the fixes are covered at the unit/integration level (above) instead. Flagging per repo convention rather than silently relying on existing coverage; happy to add a scenario if maintainers want dedicated e2e coverage for the launcher hub screen.GPU hardware testing
Not run against real GPU hardware in this environment (no
/dev/kfdavailable); the amd-smi-detection fix is validated via the fake-binary regression test described above, which exercises the real detection/adoption code path.