Stop ImageWatcher per-file processing from blocking the event loop - #798
Open
thusser wants to merge 1 commit into
Open
Stop ImageWatcher per-file processing from blocking the event loop#798thusser wants to merge 1 commit into
thusser wants to merge 1 commit into
Conversation
ImageWatcher._worker parses every watched file with astropy's synchronous fits.HDUList.fromstring and reads/writes/removes files through LocalFile's async-declared-but-synchronous methods directly on the module's event loop, freezing RPC handling, inotify processing, and comm keepalive for seconds at a time -- the 2026-08-20 "Event loop stalled" incident on MONET South. Offload the FITS parse via asyncio.to_thread, and route all blocking LocalFile I/O (open/read/write/close/remove/find/exists) through the default executor, mirroring the existing listdir pattern. The actual open()/makedirs moves out of __init__ into an overridden __aenter__ (routed through a module-level _open_sync helper), closing the network-mounted-watch-path gap; path validation still raises synchronously at construction time. Every VFS consumer inherits the fix, not just the imagewatcher. Add heartbeat-style tests proving the event loop stays responsive during a slow FITS parse and slow LocalFile I/O. See specs/plans/2026-08-20-imagewatcher-event-loop-blocking.md for the full design.
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.
Implements
specs/plans/2026-08-20-imagewatcher-event-loop-blocking.md(Status staysproposed— the plan's own checklist flips it toimplementedonly once landed and verified at the site over one night).Problem
ImageWatcher._workerprocesses every watched file with synchronous, never-yielding operations directly on the module's single event loop:fits.HDUList.fromstring(data)— a synchronous, CPU-bound astropy parse per file.LocalFile.read/write/close/remove/exists/find— declaredasync defbut execute plain blocking syscalls with no executor (listdiralready offloads; the others never got the treatment).LocalFile.__init__opens the file synchronously, andVirtualFileSystem.open_filehas noawaitpoint before that — so even with read/write/remove offloaded, a slow/network-mounted watch path could still stall the loop at open time.Result: RPC replies, inotify handling, and comm keepalive freeze for seconds at a time (the 2026-08-20 MONET South incident,
module.py:205/207"Event loop stalled for 0.81s" / 2.81 s total).Changes
pyobs/modules/image/imagewatcher.py: offload the FITS parse viaasyncio.to_threadin_worker; add docstring notes thatprocess_extra/cleanup_extrarun on the event loop and must not block.pyobs/vfs/localfile.py: routeread,write,close,remove,find,existsthrough the default executor (module-level_find_sync/_remove_synchelpers;existscallsos.path.existsdirectly), mirroring the existinglistdirpattern. Move theopen()/makedirswork out of__init__into an overridden__aenter__via a_open_synchelper — path validation still raises synchronously at construction; every call site already usesasync with, so nothing observes the open moving one step later. All VFS consumers inherit the fix.tests/modules/image/test_imagewatcher.py(slow FITS parse) andtests/vfs/test_localfile.py(slow read/write/open via a fake slowfdand a monkeypatched_open_sync). Verified they fail on the old code and pass on the new.Verification
pytest tests/modules/image/ tests/vfs/→ 34 passed, 1 skipped (pre-existing SSH-disabled skip)./opt/pyobs/storage, read-only~/.config/sunpy, missingpyobsconsole script in the venv).ruffclean,blackclean,pyrefly0 errors on touched files.Out of scope (per plan)
Archive upload latency / queue backlog (already async, never freezes the loop) and step 0's live site measurement are flagged for follow-up in the plan.