fix(motion): keep recorded-move loading off the daemon event loop - #1358
Open
Dariushuangg wants to merge 1 commit into
Open
Dariushuangg wants to merge 1 commit into
Dariushuangg wants to merge 1 commit into
Conversation
Playing a recorded move rebuilt RecordedMoves on every request. Its constructor calls snapshot_download, which falls back to a networked download when the HF cache is cold - a blocking call inside an async route, so it stalled the uvicorn event loop. Where huggingface.co is unreachable that fallback never returns quickly, and a single emotion took down every other endpoint and the state stream with it: /api/daemon/status, /api/state/full and the WebSocket all stop answering, so clients report the robot as disconnected rather than surfacing a failed download. Load libraries through a memoized get_recorded_moves() and await it via asyncio.to_thread in both HTTP routes, matching what the WebRTC data-channel path already did. A per-dataset lock collapses concurrent cold-cache requests into one download instead of one per request, and the memoization stops a warm library being re-parsed on every playback. Assisted-by: Claude:claude-opus-5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
|
my robot is purchase and run by Seeed Studio, which is located in China. That's why there's hugging face reachability problem. |
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
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.
Issue
No issue filed yet, so the problem is described here per the template.
Happy to split it out if you'd rather track it separately.
Playing any recorded move on a robot that can't reach huggingface.co takes the
whole daemon down. The client sees a disconnect, not a failed download.
play_recorded_move_datasetbuildsRecordedMoves(dataset_name)per request.The constructor calls
snapshot_download(local_files_only=True)and, onLocalEntryNotFoundError, falls back to a networkedsnapshot_download().That is a synchronous call inside an
async defroute, so it blocks the uvicornevent loop:
/api/daemon/status,/api/state/fulland the state WebSocket allstop answering until it returns.
Reproduce on a robot with an empty HF cache and huggingface.co blocked (or just
firewall it):
The cache is empty in the first place because the wireless launcher starts the
daemon without
--preload-datasets, sopreload_default_datasets()never runs,and its failures are swallowed with a warning either way.
Description
Load libraries through a memoized
get_recorded_moves()andawaitit viaasyncio.to_threadin both HTTP routes._async_play_recorded_move(the WebRTCdata-channel path) already did exactly this — the two HTTP routes never got the
same treatment.
Three effects:
but the daemon keeps serving everything else, so a blocked HF surfaces as one
failed move instead of a dead robot.
reads 172 files; it now happens once per dataset.
collapses N in-flight requests into a single build; other datasets stay
unblocked.
Deliberately not changed here, to keep this to one concern — happy to open
follow-ups:
alternative is cache-only playback returning 503, but that regresses robots
that legitimately download on first use.
--preload-datasetsstill isn't passed by the wireless launcher.Anne-Charlotte/music(surfaced in the desktop app's Dances tab) isn't inDEFAULT_DATASETS, so it is never preloaded.Testing
tests/unit_tests/test_recorded_moves_cache.py, 3 cases, no hardware ornetwork (
snapshot_downloadstubbed):No route signatures or response models changed, so
docs/source/API/openapi.jsonis unaffected.
Scope of the hardware evidence: the failure below was measured on a physical
Reachy Mini Wireless, and the recovery numbers come from warming that robot's HF
cache. This patch itself has not been run on the robot — it is covered by the
unit tests above. Flagging that explicitly so a reviewer can decide whether to
ask for an on-robot run before merging.
Tested on
--sim)--mockup-sim)AI assistance
Annex — measurements on a physical robot
Reachy Mini Wireless, daemon 1.8.3, Debian 13 aarch64, CN network (mobile ISP).
HF reachability, measured from the robot over SSH (not from the dev machine —
a local TUN proxy silently tunnels the dev machine's traffic and reports a false
success):
DNS returns a Facebook-owned IPv6 block, so the connection never establishes.
~/.cache/huggingfacedid not exist on the robot.Before — cold cache, HF unreachable:
After — same robot, cache warmed via
HF_ENDPOINT=https://hf-mirror.com(emotions 172 files / 8.6M, dances 21 files / 484K):
The "after" figures are with a warm cache and therefore show the path this patch
keeps fast; the patch's distinct contribution is that the "before" row no longer
takes the other two endpoints down with it.