Fix remote HTTP test stalls and tune HTTP HDF5 caching - #784
Conversation
The remote HTTP tests intermittently stalled for 15+ seconds (or hung outright without pytest-timeout) under CPU contention. Two localhost fixture servers were single-threaded, so one slow or abandoned connection parked the accept loop and left new connections stuck in TCP SYN retry backoff. Both fixtures now use ThreadingHTTPServer like the existing http_das_path fixture. Remote HDF5 opens over HTTP also amplified transfers: h5py's metadata probe alternates between the file header and footer, and fsspec's default single-window BytesCache refetched a multi-MB block (or the entire file on range-less servers) on every jump - over 18 MB fetched to probe a 12.7 MB file. HTTP now gets the same tuned block size as S3 with a block LRU cache (blockcache) that keeps both file ends resident.
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughRemote HDF5 opening now pauses garbage collection for loop-backed and remote handles, uses bounded HTTP block caching, improves failure cleanup and abort semantics, expands HTTP regression coverage, and enables network tests on Windows. ChangesRemote HDF5 lifecycle hardening
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_utils/test_io_utils.py (1)
409-440: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover HTTPS in this regression test.
The implementation and constants now support both
httpandhttps, but this test exercises onlyhttp://. Parameterize the scheme so HTTPS regressions cannot pass unnoticed.Proposed test update
- def test_h5_reader_uses_block_cache_for_http_upath(self, monkeypatch): + `@pytest.mark.parametrize`("scheme", ("http", "https")) + def test_h5_reader_uses_block_cache_for_http_upath(self, monkeypatch, scheme): ... - path = UPath("http://example.com/example.h5") + path = UPath(f"{scheme}://example.com/example.h5")As per PR objectives, HTTP and HTTPS HDF5 caching are both in scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_utils/test_io_utils.py` around lines 409 - 440, Parameterize test_h5_reader_uses_block_cache_for_http_upath over both “http” and “https” schemes, and construct the UPath URL from that parameter. Keep the existing monkeypatches, configuration, and block-cache assertions unchanged so the regression test covers both supported protocols.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_utils/test_io_utils.py`:
- Around line 409-440: Parameterize
test_h5_reader_uses_block_cache_for_http_upath over both “http” and “https”
schemes, and construct the UPath URL from that parameter. Keep the existing
monkeypatches, configuration, and block-cache assertions unchanged so the
regression test covers both supported protocols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bd3f303-be17-44cd-a670-ca695eb36be5
📒 Files selected for processing (4)
dascore/constants.pydascore/utils/hdf5.pytests/test_io/conftest.pytests/test_utils/test_io_utils.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ea86f396c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # alternates between the file header and footer, and fsspec's | ||
| # default single-window cache refetches a full block (or the whole | ||
| # file on range-less servers) on every jump. | ||
| cache_type = "blockcache" if protocol in ("http", "https") else "readahead" |
There was a problem hiding this comment.
When an HTTP HDF5 read spans at least 32 blocks, fsspec's blockcache retains its default maximum of 32 blocks; with DASCore's default 5 MiB block size, each open remote file can therefore hold roughly 160 MiB of cached data until the handle closes. Large DAS files—or multiple concurrent readers—can cause a substantial memory regression compared with the previous single-window cache. Pass cache_options with a small maxblocks value sufficient to retain the header and footer rather than accepting fsspec's default.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #784 +/- ##
==========================================
- Coverage 99.98% 99.97% -0.02%
==========================================
Files 164 164
Lines 17707 17783 +76
==========================================
+ Hits 17705 17779 +74
- Misses 2 4 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Seeking a streaming (size-less) HTTP file raises a different fsspec error than the no-range probe; match it in is_no_range_http_error so such servers also fall back to a cached local copy. Cap the HTTP block cache at 8 blocks so one handle cannot retain unbounded fetched data.
|
Note on scope: this fixes the reproducible stall class (accept-loop parking — before: 2/3 pinned runs of the module hit the 30 s pytest-timeout; after: 5/5 stable) and the transfer amplification, but one intermittent stall remains in |
Root cause of the long-standing intermittent stall in remote HDF5 reads (the TODO in test_http_range_hdf5_read_succeeds): h5py holds its global phil lock while a remote fileobj read blocks on fsspec's event-loop thread, and an automatic garbage collection triggered on that thread must acquire phil to deallocate dead h5py objects. Main thread holds phil waiting on the loop; the loop's GC waits on phil - deadlock. Diagnosed with faulthandler burst dumps during a live stall (loop thread frozen at an allocation point with the GC flag set, zero CPU) and confirmed causally: gc.disable() eliminated the stall in 6/6 pinned runs, and gc callbacks show collections running on the fsspecIO thread exactly during range fetches. Remote h5py handles now pause automatic collection for their (bounded) lifetime, resuming on close - the mitigation h5py's file-object docs recommend. Existing cyclic garbage is collected before each pause, the pause is BaseException-safe, close() claims teardown atomically, and a __del__ backstop covers leaked handles. The in-test skip_on_timeout band-aid is removed so CI exercises the fixed path.
|
Update: the residual stall is root-caused and fixed in ba6ba5c — it was never a network problem. It's a platform-agnostic ABBA deadlock: h5py holds its global Evidence: faulthandler burst dumps during a live stall show the loop thread frozen at an allocation point with Fix: remote h5py handles pause automatic collection for their bounded lifetime (resumed on close) — the mitigation h5py's file-object docs recommend for exactly this pattern. Pre-pause |
Three independent adversarial reviews of the GC-pause deadlock fix surfaced real gaps, all addressed here: - The io.IOBase branch of open_h5_resource bypassed the pause, so a user-supplied fsspec file object hit the original deadlock through a different door. Loop-backed file objects (fsspec async filesystems) now take the same paused path. - _type_caster leaked the handle it opened when the wrapped FiberIO method raised, leaving collection paused indefinitely if the exception was retained. The except branch now closes created handles. - IOResourceManager.__exit__ committed unconditionally, so a mid-write exception uploaded a partial file to remote targets; on error it now aborts handles that support it. - pause_gc's maintenance collect moves outside the lock (a finalized leaked handle's close -> resume_gc would self-deadlock on the non-reentrant lock), runs unconditionally so leaked handles always self-heal on a later open, and is rate-limited to bound its cost when opening many remote files. - Interrupt-safety: pause increments depth before disabling and resume re-enables before decrementing, so a KeyboardInterrupt between the two can no longer leave collection off permanently. - A fork handler resets the pause state in child processes, matching the existing _reinit_after_fork pattern. - Documented the load-bearing one-way reference invariant (nothing on the loop thread may reference h5py objects) in _FallbackFileObj. The review also verified the fixture stack is Windows-correct and the old skip's stated cause is fixed, so the win32 skip is removed and windows-latest joins the (non-gating) network_tests matrix as a canary.
|
Ran three independent adversarial reviews on the GC-pause mechanism (lock-ordering, CPython GC semantics, handle lifecycle/platforms) — commit 6a0a8ef addresses everything they surfaced: Real gaps fixed:
Verified sound by the reviews (now documented in code): refcount-driven h5py deallocs can never land on the loop thread because references flow strictly one way through the sync bridge; the implementation matches h5py's file-object docs' recommended mitigation; wasm and free-threaded CPython carry over. Windows: the review confirmed the fixture stack is Windows-correct and the skip's stated cause is the now-fixed platform-agnostic deadlock, so the win32 skip is removed and |
The fork handler only runs in forked children and the loop-backed fileobj constructor-failure path only on broken files, so neither was exercised; call them directly. The codecov coords.py delta is a line-skew artifact (one flagged line is a comment); the file is fully covered locally on this branch.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.xdg-test/pint/01448202a267fcbe1a419a34e97f9f53346f5212.json:
- Line 1: Replace the machine-specific absolute source paths in the committed
Pint cache metadata with portable identifiers, or regenerate all entries in CI.
Apply this to .xdg-test/pint/01448202a267fcbe1a419a34e97f9f53346f5212.json:1-1,
.xdg-test/pint/a4391b0ab5f2b7b7b67c1b2c28cd86d2cb5b3dec.json:1-1,
.xdg-test/pint/c2a57dde47058a3a44a02d7d48817affe593f48e.json:1-1, and
.xdg-test/pint/db8943bf1adb3e18852fbc22864ab561c4a6be8c.json:1-1, preserving the
respective constants_en.txt and default_en.txt metadata.
In `@dascore/utils/io.py`:
- Around line 247-258: Update close_all to isolate cleanup failures per cached
handle: wrap each handle’s abort or close operation in exception handling so one
failure does not stop iteration through self._cache. Continue attempting every
remaining handle, while preserving the existing abort selection and normal close
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20e16382-14c6-491b-8f48-3c212c63f518
⛔ Files ignored due to path filters (6)
.xdg-test/pint/01448202a267fcbe1a419a34e97f9f53346f5212.pickleis excluded by!**/*.pickle.xdg-test/pint/0ffe33abec788d9f5d1f14291461e7bed27e2a4f.pickleis excluded by!**/*.pickle.xdg-test/pint/a4391b0ab5f2b7b7b67c1b2c28cd86d2cb5b3dec.pickleis excluded by!**/*.pickle.xdg-test/pint/c2a57dde47058a3a44a02d7d48817affe593f48e.pickleis excluded by!**/*.pickle.xdg-test/pint/cfc24d8be1b723f7021ad520e66c8ae0e35e7152.pickleis excluded by!**/*.pickle.xdg-test/pint/db8943bf1adb3e18852fbc22864ab561c4a6be8c.pickleis excluded by!**/*.pickle
📒 Files selected for processing (15)
.github/workflows/runtests.yml.xdg-test/dascore/data/0.0.0/terra15_das_1_trimmed.hdf5.xdg-test/pint/01448202a267fcbe1a419a34e97f9f53346f5212.json.xdg-test/pint/0ffe33abec788d9f5d1f14291461e7bed27e2a4f.json.xdg-test/pint/a4391b0ab5f2b7b7b67c1b2c28cd86d2cb5b3dec.json.xdg-test/pint/c2a57dde47058a3a44a02d7d48817affe593f48e.json.xdg-test/pint/cfc24d8be1b723f7021ad520e66c8ae0e35e7152.json.xdg-test/pint/db8943bf1adb3e18852fbc22864ab561c4a6be8c.jsondascore/io/core.pydascore/utils/hdf5.pydascore/utils/io.pydascore/utils/remote_io.pytests/test_io/test_io_core.pytests/test_io/test_remote_http.pytests/test_utils/test_io_utils.py
One cached handle raising in IOResourceManager.close_all no longer skips cleanup of the remaining handles (remote handles resume garbage collection in close); the first error re-raises after all handles were attempted, and __del__ suppresses it. Also remove the accidentally committed .xdg-test cache directory. The block-cache memory concern was already addressed by the maxblocks cap in an earlier commit.
Two adversarial reviews of the pause found six ways it could leave automatic collection disabled for the rest of the process, or lift it while a remote read was still running. All are fixed, and the machinery around it is smaller than before. Fixed: - A BaseException from the owned file object's close skipped the resume entirely, with _closed already set so no retry and no __del__ could recover it. CancelledError out of fsspec, or Ctrl-C during a network close, was enough. The resume now sits in its own finally. - A handle inherited through a fork resumed the child's own pause when closed there, re-enabling collection while the child was mid-read. Handles now record the pid that paused for them. - _is_loop_backed_fileobj missed wrapped objects: io.BufferedReader over an fsspec HTTP file reads through the loop thread but has no .fs, so it took the unpaused branch with the deadlock window open. The check now unwraps a few levels, and duck-types async_impl instead of importing fsspec.asyn. - _type_caster caught Exception, so Ctrl-C inside a FiberIO read leaked the handle it opened; the pause then lasted as long as the traceback, which under pytest or a REPL is the session. - An interrupt between constructing the wrapper and clearing owns_pause could resume twice. Ownership is no longer expressed by a flag. - The docstring claimed nothing could suppress collection indefinitely. A handle that is never closed does exactly that; it now says so. Removed: the owns_pause dance duplicated at both call sites (one _open_h5_paused helper), the per-handle close lock (an atomic dict.pop gives resume-exactly-once), one of the two rate-limit globals, and the sys.modules lookup. The previous tests covered bookkeeping rather than the property: 93 of 97 passed with the pause removed entirely. The new ones model the real cycle - a lock-holding reader waiting on a thread whose collection needs that lock - and fail without it. Each of the seven breaks above was mutated back in and confirmed caught.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dascore/utils/hdf5.py (1)
130-137: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse explicit
is not Nonewhen picking the wrapped layer.
getattr(resource, "raw", None) or getattr(resource, "buffer", None)relies on truthiness. A file-like wrapper that defines__bool__/__len__(falsy when empty) would be skipped, silently failing detection and reopening the deadlock window this function exists to close.♻️ Proposed change
- wrapped = getattr(resource, "raw", None) or getattr(resource, "buffer", None) + wrapped = getattr(resource, "raw", None) + if wrapped is None: + wrapped = getattr(resource, "buffer", None) if wrapped is None or wrapped is resource: return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dascore/utils/hdf5.py` around lines 130 - 137, Update the wrapped-layer selection in the resource traversal loop to prefer resource.raw whenever it is not None, otherwise use resource.buffer, without relying on truthiness. Preserve the existing None and self-reference termination checks and async_impl detection.dascore/utils/io.py (1)
264-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the intentional broad cleanup catch.
Ruff reports BLE001 here. The broad catch is justified because cleanup must continue for every handle, but add a narrowly scoped
# noqa: BLE001with rationale (or use a shared cleanup exception) so this does not become a CI lint failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dascore/utils/io.py` around lines 264 - 265, Update the broad exception handler in the cleanup loop around first_exc to add a narrowly scoped # noqa: BLE001 with a concise rationale explaining that cleanup must continue across all handles; preserve the existing first exception capture behavior and avoid broader lint suppressions.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dascore/utils/hdf5.py`:
- Around line 151-155: Update the exception cleanup block surrounding the
visible fileobj.close() call so resume_gc() executes in a finally clause even
when close raises BaseException. Mirror the nested try/finally cleanup structure
used by _ManagedH5pyFile.close(), while preserving the existing re-raise
behavior.
---
Nitpick comments:
In `@dascore/utils/hdf5.py`:
- Around line 130-137: Update the wrapped-layer selection in the resource
traversal loop to prefer resource.raw whenever it is not None, otherwise use
resource.buffer, without relying on truthiness. Preserve the existing None and
self-reference termination checks and async_impl detection.
In `@dascore/utils/io.py`:
- Around line 264-265: Update the broad exception handler in the cleanup loop
around first_exc to add a narrowly scoped # noqa: BLE001 with a concise
rationale explaining that cleanup must continue across all handles; preserve the
existing first exception capture behavior and avoid broader lint suppressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e8d1ed25-c220-4aa9-9f7b-46880f9f6bd3
📒 Files selected for processing (6)
dascore/io/core.pydascore/utils/hdf5.pydascore/utils/io.pydascore/utils/remote_io.pytests/test_utils/test_gc_pause.pytests/test_utils/test_io_utils.py
🚧 Files skipped from review as they are similar to previous changes (2)
- dascore/utils/remote_io.py
- tests/test_utils/test_io_utils.py
| except BaseException: | ||
| with suppress(Exception): | ||
| fileobj.close() | ||
| resume_gc() | ||
| raise |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
resume_gc() can be skipped if fileobj.close() raises a BaseException.
suppress(Exception) doesn't cover KeyboardInterrupt/SystemExit, so an interrupt during teardown strands the pause with automatic collection off process-wide. _ManagedH5pyFile.close() already guards this with a nested try/finally; mirror it here.
🛡️ Proposed fix
except BaseException:
- with suppress(Exception):
- fileobj.close()
- resume_gc()
- raise
+ try:
+ with suppress(Exception):
+ fileobj.close()
+ finally:
+ resume_gc()
+ raise📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except BaseException: | |
| with suppress(Exception): | |
| fileobj.close() | |
| resume_gc() | |
| raise | |
| except BaseException: | |
| try: | |
| with suppress(Exception): | |
| fileobj.close() | |
| finally: | |
| resume_gc() | |
| raise |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dascore/utils/hdf5.py` around lines 151 - 155, Update the exception cleanup
block surrounding the visible fileobj.close() call so resume_gc() executes in a
finally clause even when close raises BaseException. Mirror the nested
try/finally cleanup structure used by _ManagedH5pyFile.close(), while preserving
the existing re-raise behavior.
Both sides rewrote the remote IO layer, so the interesting part is IOResourceManager.close_all: dev added the instance lock, this branch added abort=True and per-handle error isolation (one handle failing must not skip the others, since remote handles resume the GC pause in close). The merge keeps all three. Everything else auto-merged and was checked rather than assumed: both fork handlers are present, dev's lru_cache/per-resource download locks sit alongside this branch's GC pause, and __exit__ still aborts uncommitted writes on error.
The branch grew through several review rounds; this pass is net -86 lines while fixing two things the rounds left behind. close_all caught Exception, so a KeyboardInterrupt from one handle's close skipped every handle after it - the same leak _type_caster was widened to BaseException to prevent, and remote handles resume the GC pause in close. It now catches BaseException for that reason. clear_cache dropped its cache only when close_all returned, so a failing handle left closed handles cached; the clear moved to a finally. pause_gc claimed its rate-limit slot outside the lock, so two openers could each run the full collect. The claim moved inside; gc.collect() stays outside, since finalizing a leaked handle calls resume_gc, which takes that lock. Removed, without losing coverage: - The GC pause tests in test_io_utils duplicated test_gc_pause, which is the file that actually models the lock cycle (the older bookkeeping tests passed with the pause removed entirely). - The S3 and HTTP open-kwargs tests differed only in the expected cache type, so they are one parametrized test. - _DummyHandle was defined four times and the fsspec async filesystem fake twice; both are module level now. The fake still subclasses the real AsyncFileSystem, so the async_impl duck-type stays validated against fsspec rather than against another fake. - The three localhost HTTP fixtures were identical apart from handler and root, and now share one _serve_das_tree context manager. http/https was spelled three times (constants, remote_io, hdf5); it is one constants.http_protocols that the tuned-protocol tuple builds on. Changelog entries for the two user-visible behaviors: remote writes abort rather than commit a partial upload on error, and remote HDF5 reads pause automatic garbage collection while a handle is open.
1 participant
Description
Tracks down the intermittent remote-HTTP deadlock/stall noted in the
test_remote_http.pymodule docstring (the Windows skip rationale). Pinning the suite to 2 CPUs made it reproducible, which separated it into three independent causes.1. Single-threaded fixture servers (test-only)
Two localhost fixture servers (
http_regression_das_path,http_range_das_path) were single-threadedHTTPServers. One slow or abandoned connection parks the accept loop, and every new connection then sits in TCP SYN retry backoff — observed as reproducible 15 s stalls (the SYN retry schedule), 30 s pytest-timeout failures, or an indefinite hang when pytest-timeout isn't installed. Both fixtures now useThreadingHTTPServerlike the existinghttp_das_pathfixture. Before: 2/3 pinned module runs hit the 30 s timeout; after: 5/5 stable.2. A real deadlock between h5py's lock and garbage collection (product bug)
h5py holds its process-global lock while blocking on fsspec's event-loop thread for each remote fetch. Python's cyclic garbage collector can fire on any thread at any time — including that loop thread — and deallocating a dead h5py object there needs the same lock. So the loop thread waits on the lock while h5py waits on the loop thread, and neither moves.
This is platform-agnostic. Windows simply lost the timing coin flip more often, which is why it was previously filed as Windows flakiness. Remote h5py handles now pause automatic collection for the handle's lifetime (
dascore.utils.remote_io.pause_gc) and resume it on close; reference counting still frees non-cyclic garbage.The pause is balanced across the cases that can strand it: a
BaseExceptionraised during teardown, a handle inherited through a fork (handles record the pid that paused for them, so a child cannot resume a pause it never took), an interrupt inside aFiberIOread, and buffered wrappers that hide the fsspec filesystem from the loop-backed check. A rate-limitedgc.collect()on the next remote open recovers a handle leaked inside a reference cycle, which__del__cannot reach while collection is off.Because this is fixed, the Windows skip is removed and
windows-latestjoins thenetwork_testsmatrix — Linux, macOS, and Windows now all exercise the localhost-HTTP path. That job iscontinue-on-error, so it reports without gating unrelated changes.Note that the pause is process-global for the lifetime of an open remote handle: cyclic garbage from every thread accumulates until the last one closes. That is a deliberate trade against a hard deadlock, and it is documented on
pause_gc.3. HTTP HDF5 reads overfetched (performance)
Remote HDF5 opens over HTTP amplify transfers: h5py's metadata probe alternates between the file header and footer, and fsspec's default single-window
BytesCacherefetches a multi-MB block on every jump — debug logs showed 18+ MB fetched to probe a 12.7 MB file, and range-less servers re-stream the entire file per miss.H5Reader._get_open_kwargsalready tuned S3 for exactly this reason; HTTP now gets the tuned block size too, withcache_type="blockcache"(a block LRU that keeps both file ends resident). Added a unit test mirroring the existing S3 one.Separately, servers that report no size stream rather than range, so
is_no_range_http_errornow also routes "cannot seek streaming HTTP file" to the same local-file fallback.Also here
dc.writeto a remote path no longer commits a partial upload when the write fails —IOResourceManager.__exit__aborts uncommitted work on error instead of closing it.close_allisolates per-handle failures so one bad close cannot skip the rest, since remote handles resume the GC pause inclose.Checklist
I have (if applicable):
Summary by CodeRabbit
New Features
Bug Fixes
Tests