Fsspec with block granularity - #701
Merged
Merged
Conversation
Staged design notes for letting blosc2.open() accept fsspec URLs, so containers can live in S3, GCS, Azure, archives or memory without the caller downloading them first. Three phases, each shippable alone: a whole-object read via from_cframe, a local filecache layer that restores full format coverage and mmap, and byte-range access. The recommendation is to ship phase 1 and wait, since nearly every open design question belongs to the caching layer rather than to the feature itself. Records what was verified while scoping rather than assumed: a .b2nd file is a contiguous frame that from_cframe reconstructs; fsspec's filecache hands back a local path blosc2.open() already accepts; frame.c routes every read through blosc2_io_cb, so byte-range access needs no c-blosc2 change, though third-party backends re-open per lazy block and the callbacks run on the worker threads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blosc2.open(), save_array() and save_tensor() now accept any fsspec URL (s3://, gs://, zip://, memory://, and chained ones), behind a new optional [fsspec] extra. The container is transferred whole in one GET/PUT, which covers single-file containers in read mode; .b2d directories, offset != 0 and mode != 'r' raise NotImplementedError. The write branch lives in pack_tensor() so that save_array, save_tensor and pack_array2 all inherit it from one place. file:// and http(s):// keep their existing routes, the latter to C2Array. Protocol drivers (s3fs, gcsfs...) and their credentials stay the caller's concern: fsspec already names the missing package, so we do not mirror that table. Phases 2 (local cache) and 3 (byte-range chunk access) of the plan stay unstarted, awaiting a user who hits the in-memory ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blosc2.open(url, cache_storage=dir) downloads the container into dir and opens it as an ordinary local path, so directory containers (.b2d stores, sparse frames), offset and mmap_mode all work -- everything the whole-object in-memory read cannot do. Repeated opens then cost a staleness check instead of a transfer. Single files ride fsspec's filecache, with check_files=True: fsspec does not verify staleness by default and happily served a cached array whose remote bytes had changed. Directory containers have no such layer in fsspec, so the prefix is fetched whole and re-fetched whenever the remote listing stops matching a JSON manifest written at download time. Caching stays opt-in with no default location: an implicit cache filling a disk with multi-GB arrays is not a good surprise. Write-back is still out of scope, as is phase 3 of the plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blosc2.open(url, lazy=True) leaves the container in the object store and returns a Proxy that fetches only the chunks a slice touches, one range read each. On a 36 KB frame over memory://, opening costs 276 bytes and a 50-element slice 2 KB. This is route 3a of the plan, which it had parked behind 3b's I/O-callback bridge on the grounds that we have no way to get chunk offsets out of a cframe. We do: the frame header is a msgpack array, so unpacking it yields header_len, the compressed size and the b2nd metalayer (shape, chunks, blocks, dtype) with exactly one field -- header_len itself -- located by hand. The offsets are a Blosc2 chunk at header_len + compressed_size, relative to the end of the header, and a negative offset is a run-length chunk that was never written and is rebuilt locally. FsspecNDSource is exported, so a Proxy over it can be given a persistent cache file. aget_chunk overlaps fetches on async backends, which is where the S3 win is; memory:// is not async, so only the blocking fallback is covered by tests. Contiguous frames holding an NDArray only: plain SChunks, sparse frames and .b2d stores raise and point at cache_storage=. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_http_still_goes_to_c2array asserted FileNotFoundError, which is not C2Array routing: a bare http(s) URL is not a C2Array at all, since that path is entered through blosc2.URLPath. The invariant being tested is that http(s) never reaches fsspec, so say that instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Proxy(src, urlpath=..., mode="a") always called blosc2.empty() on that path, so a cache only worked the first time: on the next run the file was there and the constructor died with "Could not build empty array". That made a persistent chunk cache -- the whole point of pointing a proxy at a file -- reachable only through the private _cache= escape hatch. Now mode="a" over an existing container adopts it, and chunks fetched by an earlier run are not fetched again. The reopen goes through blosc2_ext.open rather than blosc2.open, which would try to rebuild the source we already hold and raises outright for sources it cannot reconstruct from the cache metadata. Two guards, because silently serving the wrong bytes is worse than failing: the container must carry the proxy-source metalayer, and its shape and dtype must match the source. Anything else raises instead of being reused or overwritten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fsspec section had grown a 30-line API tutorial -- three usage modes, five code blocks, a note on cache lifetimes -- one commit at a time, on a page whose other extras get a table row and a pointer. All of it already lives in blosc2.open's docstring and the FsspecNDSource reference, so cut it to the table row, the pip recipe with the backend, and one sentence naming the three modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same accretion as the install page, in three more places. open()'s Notes had grown three fsspec bullets restating what the lazy= and cache_storage= kwargs entries already said, in a section whose other bullets are three lines; the release notes had four bullets for what a reader experiences as one feature plus one fix; and fsspecndsource.rst repeated the class docstring that autoclass renders right below it. Each fact now lives where a reader would look for it: the kwargs entries for what an option does, one Notes bullet for the install and read-only caveats, the class docstring for the persistent-cache recipe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
They were mutually exclusive, which made the persistent chunk cache reachable only by building the Proxy by hand -- open() cannot forward urlpath= to it, that being open()'s own parameter. But the two knobs answer different questions: cache_storage says where this container's local copy lives, lazy says whether that copy is the whole thing or just the chunks touched so far. So blosc2.open(url, lazy=True, cache_storage=dir) now keeps the fetched chunks in a container under dir, and a later run starts from them. The cache is stamped with the remote size and mtime and discarded when they change. A stale chunk cache is worse here than in the whole-object case: the chunks were fetched by offsets read from a frame that no longer exists, so they are not old data but wrong data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The stamp was [size, mtime or LastModified or ""], which on memory:// -- the only backend the tests use -- collapses to size alone, since it exposes neither key. A frame replaced by one of identical size was therefore served from the stale cache: chunks fetched at offsets read from a file that no longer exists. The existing test missed it because compression made the two frames different sizes. fs.ukey() is fsspec's own token for "these exact bytes", the one filecache checks, so it works off whatever each backend actually exposes instead of the fields we guessed it would. The test now writes both frames uncompressed so their sizes match and only a real content check can pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The write side only covered save_array/save_tensor, so blosc2.save() and NDArray.save() -- the natural calls for a container that already exists -- died with "Error while copying the array": they route through copy(), and the C writer cannot target a URL. They now build the cframe and upload it as a single object, honouring cparams/chunks kwargs by copying in memory first. contiguous=False raises, a sparse frame being a directory. Constructors given a URL (zeros, asarray, copy, SChunk) cannot work at all: the C layer rewrites a frame's header and offsets as chunks land, and an object store has no partial write. They used to fail as "Could not build zeros array" from deep in C; Storage and SChunk.__init__ now reject the URL up front and name save() as the way to do it. Whole-object replace is the only shape a remote write takes here, so two writers to the same key silently lose one. Said in the docstring rather than left to be discovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers the three read modes and the write, over memory:// so it runs with no network, no credentials and no protocol driver -- with the s3:// form in a comment, since swapping the URL is the only change needed. Ends on a LazyExpr over the lazy handle, which is the property worth showing: slicing an expression built on a remote array still fetches only the chunks that slice needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One line each on the install page and the FsspecNDSource reference, in the style random.rst already uses for random-constructor.py. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get_chunk cost two reads: sixteen bytes for the chunk header, to learn the compressed size, then the chunk itself. Bounding the read by whatever is stored next -- the following chunk in file order, or the offsets chunk -- and truncating locally to the size in the header does it in one, halving the round trips a slice costs against an object store. The bound is capped at what a chunk can weigh, so the hole an updated chunk leaves behind cannot turn into an absurd read, and it comes from the sorted offsets rather than the next index, since a rewritten chunk is appended at the end and leaves the offsets non-ascending. Covered by a test that updates a chunk before uploading. This also drops the shared file handle: the source now keeps no file position, which is what makes it safe to call get_chunk from several threads at once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ordinary slicing goes through fetch(), which pulled chunks one at a time -- so a slice spanning 12 chunks was 12 serial round trips against an object store, whatever afetch() could do. fetch() now takes max_concurrency=, and reads it from the source when the source carries one, so blosc2.open(url, lazy=True, max_concurrency=8) applies to every slice without the caller ever naming fetch(). Threads, not asyncio: driving afetch() from __getitem__ would mean asyncio.run() inside a sync method, which raises inside any running event loop -- every notebook -- and would be the first sync-over-async in the library. A thread pool works the same in a script, a notebook, and someone else's event loop, and fsspec releases the GIL on socket I/O. Cache writes stay on the calling thread; only the fetches fan out. Default 1, i.e. exactly today's behaviour, since the gain is invisible against memory:// and needs a real endpoint to justify a different default. The test proves overlap rather than timing it: each fetch waits on a two-party barrier, which only clears if another fetch is in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Serial-by-default was the inconsistent choice, not the conservative one: afetch() already used REMOTE_MAX_CONCURRENCY for remote sources, so the same source fetched eight at a time when awaited and one at a time when sliced. The speedup is still unmeasured here, but the cost of being wrong is not. Over memory://, where the pool can only lose, a 100-chunk read goes from 1.1 ms to 2.2 ms -- about 10 us per chunk, against the ~30 ms an S3 round trip costs. Pass max_concurrency=1 for a protocol with no latency to hide. Sources other than FsspecNDSource are unaffected: fetch() still reads the attribute from the source and falls back to serial, since concurrency is only safe for a thread-safe get_chunk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Shows what max_concurrency= buys: 7x on a 100-chunk read, 5x on a 12-chunk slice, and free on a repeat since the proxy caches. Nothing available offline has latency to show it with -- memory://, zip:// and tar:// are all local reads, where the pool only costs ~10 us per chunk -- and http:// is reserved for Caterva2, so a local server is not an option either. The example therefore subclasses fsspec's in-memory filesystem with a fixed 5 ms delay and says so plainly, rather than implying a benchmark it cannot run. Against a real bucket the delay is real and the code is identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One clause on the install page, one sentence on the FsspecNDSource reference, next to the max_concurrency parameter it demonstrates. The reference notes the round trip is simulated, so nobody reads the numbers as a benchmark of a real bucket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Corrects the record on the cache stamp, which the plan still described as a size-and-mtime tuple after it became fs.ukey(). Adds the two findings worth keeping. First, the tier-3 trigger fired -- phase 3 exists -- and the answer is still no moto: of the three things memory:// cannot reach, moto buys two, and blockcache::memory:// covers one of those for free. Second, and less expected, memory:// misleads by being *poorer* in metadata than any real backend, which is what let a size-only stamp serve a stale chunk cache; moto would have hidden that bug rather than caught it. The concurrency default is now backed by numbers on both sides, and the open questions collapse to one: how any of this behaves against a real endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tests/test_fsspec.py opens with importorskip("fsspec"), which is right --
the extra is optional -- but nothing in the test group pulls fsspec, so all
47 tests skipped in every CI job. The feature had zero coverage there while
looking green.
s3fs and friends stay out deliberately: the tests run on memory:// and a
local zip, and no backend installed is also the configuration most users of
the [fsspec] extra are in, so it is the one worth exercising. Excluded on
wasm32, following the other platform-gated test deps.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
max_concurrency=8 is the one number on this branch chosen by argument rather than measurement: the pool's cost was measured, the gain never was, because nothing offline has a round trip to hide. This sweeps 1..32 over a slice and a whole-array read and prints where the curve flattens. It also runs afetch(), whose async path has never executed at all -- memory:// is not an async backend, so the test suite only ever reaches its blocking fallback. Endpoint and anon options go through fsspec.config.conf, since blosc2.open() has no storage_options= passthrough; that is also the only way to point any of this at R2, B2 or MinIO today, and worth revisiting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
moto was dismissed earlier in this work, but that was about adding it as a CI test dependency. As a local endpoint for running this benchmark by hand it is the easiest of the options -- one pip install, no Docker, no binary -- and it is what s3fs's own test suite runs against. With the caveat that matters: it is a single-process Python mock with no latency, and it may serialize requests, so the sweep against it can show a flat or inverted curve while the client is behaving perfectly. It answers whether the async path runs, not how fast anything is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pointed at a live S3 endpoint for the first time (moto server + s3fs), the
async path failed on every chunk:
HTTPClientError: ... got Future <...> attached to a different loop
fsspec drives an async filesystem's coroutines on a private event loop of
its own, in a background thread. Awaiting fs._cat_file() from the caller's
loop therefore uses a client built on one loop from another, which
aiobotocore rejects. Its blocking API is the supported way in, so
aget_chunk now hands get_chunk to a worker thread: that call dispatches to
fsspec's own loop, so the thread parks on a queue rather than on a socket,
and afetch() keeps overlapping fetches as before.
memory:// cannot catch this -- it is not an async backend, so aget_chunk
took the fallback branch there and everything passed. The test now asserts
the mechanism instead of only the result: afetch must reach get_chunk, which
fails if anyone awaits the filesystem coroutine again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
memory:// cannot see the class of bug that lives on a real backend, and has now hidden two: aget_chunk awaiting an async filesystem's coroutine (fine on memory://, which is not async, and broken on every chunk against s3fs), and a cache stamp that degraded to size-only because memory:// exposes no mtime. tests/test_fsspec_s3.py runs moto in-process (ThreadedMotoServer on a free port, so xdist workers do not collide) with s3fs in front of it: a real S3 protocol, real range requests, a real async backend, and still offline -- no credentials, no network, so no `network` marker. Eight tests in ~3 s. Reverting the aget_chunk fix fails exactly the two that cover it. moto[server] and s3fs go into the test group as well as dev, so this runs on push rather than only when someone remembers. That is a heavier dependency tree on every job; if it ever churns badly enough to break installs, moving both to dev-only and running them nightly is the fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three real defects, all confirmed by repro before fixing. file:// was excluded from the fsspec branch so it could keep mmap and the directory formats, but nothing downstream stripped the scheme, so it reached os.path.exists() and the C layer as a literal filename and failed. The docstring promising otherwise was simply wrong. Normalized to a native path in open(), NDArray.save(), Storage, and the two constructor paths that bypass Storage. The directory cache manifest compared name, size and mtime, which is the same mistake already fixed for the lazy chunk cache and missed here: on a backend with no mtime -- memory://, and the tests only use memory:// -- a same-size rewrite left the manifest unchanged and served stale files. It now hashes each entry with tokenize(), which is what fs.ukey() uses. Reusing a proxy cache checked shape and dtype only. Chunk numbers are the currency between cache and source, so a same-shaped source chunked differently fetched the wrong chunks and returned wrong data with no error at all; chunks and blocks are compared now, and non-ND sources get the same check on nbytes, chunksize and typesize instead of none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All reproduced first; the first two make lazy=True unusable for whole classes of ordinary arrays. The frame header was unpacked with raw=False. Its flags field is a msgpack *string* holding four raw bytes, and clevel rides in the high nibble of one of them, so from clevel=8 up it is not valid UTF-8 and every lazy open died with UnicodeDecodeError. Unpacked raw now. _special_chunk rebuilt run-length chunks with compress2 and no blocksize, which makes blosc2 take the whole chunk. Whenever blocks != chunks -- the default for a large chunk -- the cache rejected the chunk with "Error while getting the buffer", and with cache_storage= the bad chunk was written to disk. It now passes the container's blocksize. Structured dtypes are stored as their repr, so np.dtype() on the metalayer string raised TypeError; added the ast.literal_eval fallback blosc2_ext already uses. _reopen_cache dereferenced cached.shape before it could report a kind mismatch, raising AttributeError instead of the intended ValueError. normalize_urlpath dropped the drive letter for file://C:/x, where urlparse puts it in netloc. And open()'s Notes listed .b2z among the formats a plain URL read handles: it is a zip archive, not a cframe, so it needs cache_storage like the directory formats. Tests now parametrise over clevel, over blocks != chunks and over a structured dtype, since every one of these hid behind default parameters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correctness: - A cached copy kept fsspec's plain hash as its name, so a `.b2e` store came back as a bare SChunk: `blosc2.open()` dispatches on the extension alone. - A lazy open of an empty array read the trailer as an offsets chunk and died with a decompression error. - A half-written cache left by an interrupted run was fatal to every later `lazy=True` open, rather than being discarded like a stale one. - `mode="r"` was dropped on the way to an fsspec URL in `NDArray.save()` and `pack_tensor()`, which then overwrote the object. - A `file://` URL naming a host built a relative path instead of a UNC one. - A `storage=` mapping never reached `Storage.__post_init__`, so it skipped both the `file://` normalization and the fsspec rejection. Silent no-ops, now rejected: - `max_concurrency=` outside `lazy=True`. - Constructor kwargs (`contiguous=`...) handed to a Proxy that reuses a cache. Also: read the frame index through exact ranges rather than a buffered handle, which on s3fs fetched a 50 MiB block per seek; make the cache-mismatch message name the fields it is actually comparing; and test slice membership against a set rather than scanning a numpy array per chunk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same shape, dtype and partitioning is not the same bytes: a replaced remote frame keeps its layout while every cached chunk, and every offset it was fetched by, goes stale. blosc2.open() already refetched in that case, but the hand-built form the FsspecNDSource docstring recommends went straight to the Proxy and skipped the check. So the Proxy stamps its cache with whatever identity the source can name itself by, and refuses one built against other bytes. Sources that have no identity to give are still adopted on geometry alone, as documented. This also takes over the stamping _lazy_fsspec_proxy() was doing by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Whether a chunk is already cached could not be read off the cache itself: a fetched chunk that is a run of a single value is stored as a special chunk, exactly like the empty ones blosc2.empty() leaves behind. Every such chunk was therefore refetched on every access and on every run, which for a full() array behind an fsspec URL meant a range request per chunk, forever. Keep a bitmap of the chunks brought over instead, persisted in the cache's vlmeta so a later run picks up where this one stopped. It also replaces the per-call scan over the chunk infos in fetch() and afetch(). Two smaller fixes on the way: - Say in the Proxy docstring what adopting a cache actually checks: geometry only, unless the source can name the bytes it reads as FsspecNDSource does. - Shut the fetch pool down with cancel_futures: map() queues every chunk up front, so an error (or a Ctrl-C) waited for thousands of pending requests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
file://C:/x names the host C:, which only Windows can reach, so POSIX now raises instead of silently producing a relative path. And the suffix check that routes .b2d stores to cache_storage= saw the query string, not the name; strip it (and any fragment) before comparing.
One _mark_fetched helper for the three bitmap set-bits, the partial- failure rationale lives in _save_fetched's docstring instead of two comments, and get_slice_nchunks is already unique and ordered so the set-and-sort in _missing_chunks goes.
A weakref.finalize callback (_cleanup_in_memory_store) pops from the process-global _DATA_CACHE, _SIDECAR_HANDLE_CACHE and the hot cache whenever an in-memory indexed array dies, which can be at GC time in the middle of a comprehension over one of those dicts -- and then the next iteration step raises RuntimeError: dictionary changed size during iteration. Seen on Windows CI in test_indexed_matches_unindexed. tuple()/list() copies are atomic under the GIL, and are the idiom the rest of the file already uses.
A slice of blosc2.open(url, lazy=True) pulled every byte of every chunk it touched. Blocks are the unit blosc2 already compresses independently, so it can pull only the ones the slice lands in: FsspecNDSource learns to read a chunk's block offsets and any range of it, and Proxy to ask for blocks and keep the answers. A chunk holding only some of its blocks is an ordinary chunk -- the missing ones are written as the zero-length streams the format defines as "made of zeros and stored nowhere" -- so partial chunks live in the cache with no compression, no decompression and no second container, and a persistent cache keeps them across runs. What the cache does not hold cannot be read off it, since a spliced chunk looks complete from outside, so the fetched bitmap gains one bit per block and the cache records it under its own key. Two thresholds keep the extra round trip that block offsets cost from ever being a loss, and both are decided before anything is read: a chunk under a megabyte is one cheap request anyway, and a slice wanting more than half of a chunk's blocks is asking for the chunk. So an array that does not benefit fetches exactly as it did before, and one that does gets 5-17x (measured against S3 in bench/ndarray/fsspec-block-granularity.py). Requests go out in two waves rather than two per chunk: all the block offsets, then all the blocks, sharing one pool with the chunks being taken whole. Runs of near-adjacent blocks merge into single reads, since blocks land in the file roughly but not exactly in index order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get_lazychunk() decided whether to cap the buffer at MAX_OVERHEAD by testing the special-value bits (0x70) of blosc2_flags, where the lazy flag is 0x08. So every ordinary chunk of a file-backed frame came back as 32 bytes, with its bstarts and trailer -- the block offsets and per-block compressed sizes that are the whole point of a lazy chunk -- thrown away. The 0x70 test was itself a workaround for testing 0x08 alone, which truncated the repeated value off special chunks; one test was deciding two cases. Cap only when the chunk is neither lazy nor special, so a whole chunk sitting in memory still does not get copied, which is what the cap is for. Nothing had noticed because every caller reads header fields only, and the sparse-gather path calls blosc2_schunk_get_lazychunk from C without going through this wrapper. Also guard the NULL chunk that an in-memory schunk returns for an empty data slot, which the flags read dereferenced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lazy=True fetches a whole compressed chunk per range request. Blocks are the smaller unit blosc2 already compresses independently, so a slice could fetch only the ones it touches. plans/fsspec-blocks.md works out what that would take and what it would buy. The format cooperates more than expected, and the analysis is grounded in probes rather than in the format docs: a block decodes standalone inside a synthetic one-block chunk; a chunk holding only some of its blocks, with csize == 0 zero streams standing in for the rest, is valid and update_chunk accepts it, so partial chunks can live in the cache with no compression on the fetch path; and bstarts is not monotonic (a multithreaded compressor writes blocks in completion order), so extents need the sorted-neighbour rule the chunk reader already uses. Measured against real S3 with the new benchmark, which computes exact touch ratios locally and then replays both request patterns over the network: 5-17x on arrays with multi-MB chunks, 2-5x on 1 MB chunks, and 0.5-0.7x -- one extra round trip -- on small ones or on slices that want most of their blocks anyway. Break-even is around 1 MB of compressed chunk, and it barely moves with the endpoint. Default block shapes are full in the trailing dimensions, so a column touches every block of every chunk it touches; the whole-chunk fallback is part of the design rather than a refinement of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--moto uploads the array and slices it back through blosc2.open(url, lazy=True) twice, once with blocks and once with the size threshold pushed out of reach, so what is compared is the shipped code against itself over real HTTP rather than a model of it. Its request and byte counts are exact; its times are not S3's, since moto answers over loopback in a millisecond at ~700 MB/s and the whole trade lives in the regime where a round trip is expensive and bytes are slow. So --latency-ms and --bandwidth-mbs put a stated network in front of each request, inside the proxy's thread pool where real waits would overlap. On a 39 MB array of 13 MB chunks, 134 blocks each: a point read moves 0.10 MB instead of 13.15 MB, which is 3.1x faster with an in-region network (15 ms, 90 MB/s) and 7.0x with a transatlantic one (240 ms, 3.5 MB/s) -- and 1.0x for a slice that wants every block anyway, which is the threshold doing its job. The S3 tests grow an array with chunks over the threshold, so the block path is exercised against a real async backend rather than only over memory://. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Splicing blocks into a cached chunk rewrites that chunk, so repeated fetches into the same one are quadratic in copied bytes. Cheap enough to keep -- the bytes are compressed and a rewrite never exceeds one chunk download -- but not visible from the code, so name it and name the way out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The benchmark column this claim rested on times a fetch whose layouts are known but whose blocks are not, and the implementation makes clear how rare that pairing is: layouts are read only for chunks with missing blocks, so a repeated slice, a new slice in the same session, and a repeated slice in a new session all skip that read already. What is actually on the table is one round trip for a new process reaching into a partly explored chunk, against a vlmeta blob rewritten whole on every update. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Filling a chunk block by block rewrites it on every fetch, which is quadratic in bytes moved: half of it building the chunk, half of it reading the previous one back out of the cache and taking it apart again. The write half cannot go. Deferring it to close, which is what the note here proposed, returns zeros: the cache container is what the next read comes out of, so a chunk that is not written is a chunk that is not there. The read-back half can. Proxy now keeps the blocks of the last few partly filled chunks, so the rewrite splices from what it already has. On 64 one-block fetches into a 6.89 MB chunk, 441 MB moved becomes 224 MB and 0.096s becomes 0.054s; memory is bounded by BLOCK_HOT_CHUNKS, which at 8 is what a fetch of 8 whole chunks already peaks at. Evicted chunks fall back to the read-back path, and a whole chunk arriving drops what was held about it -- both covered by tests, since neither shows up in ordinary use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It works, and it reaches the plain SChunks and other formats route A refuses. It is also ~3x slower than route A at 15 ms of latency and worse as latency grows, because c-blosc2 reads a lazy chunk's blocks in a serial loop (_blosc_getitem) with no thread pool -- which is also why the GIL worry this plan raised never materialized: the callbacks only ever run on the calling thread. Route A answers in two pooled waves; route B in N round trips. Two smaller findings, both upstream's to fix if route B is ever revived: frame_from_file_offset stat()s the urlpath before consulting the callbacks, so an io plugin cannot virtualize a path at all (the prototype needed a sparse local placeholder of the right size); and the io id ranges in blosc2.h are a trap, with a "user defined" value that does not fit the uint8 field. Code on branch fsspec-routeb-spike, kept off this one so the extension carries no unused I/O registration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
d742399 fixed this race on the Windows runner for the sites that assert the screen right after opening it: pilot.press delivers the key, but the app pushes the screen over the frames that follow, so a single pause is the race with a bigger constant. The sites that open a modal and then *drive* it with more keys were left on the old pattern, and they fail the same way with a different symptom -- the keys land somewhere else, nothing is applied, and the assertion that follows sees None. That is what CI reported: test_group_config_cached_and_reused assert None == ('region', 'mean', 'amount') where None = app._last_group _last_group is set when a group is applied and never cleared, so None means the modal sequence never landed at all. Five such sites in test_group.py and four in test_sort.py, which reaches into `app.screen.query_one("#sortby-list")` one pause after pressing S. The press("p") in test_basics.py stays as it is: it asserts a screen does *not* appear, so there is nothing to wait for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three findings from the PR review, all of which reproduce. The fetched bitmap silently broke SChunk.update_special as a cache-eviction primitive, which it has advertised since 8fc65a8 (six weeks before the bitmap landed): the evicted chunk looks exactly like one never fetched -- which is why the bitmap exists -- so the proxy went on claiming it was there and served zeros. The cache now counts the chunks it has replaced with special ones, and the proxy re-reads its bitmap when that count moves, which costs an integer compare until an eviction actually happens. vlmeta= let a caller write proxy-fetched, proxy-fetched-blocks or fsspec-stamp, handing the proxy a bitmap it never earned or an identity that makes a good cache fail its check. Those three names are refused now. And the mode="a" docstring read as though a stamped source with changed bytes was adopted, when _reopen_cache raises for exactly that; it now says which case is which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
doc/getting_started/installation.rst:63
- Remove the duplicated “or” across these two lines.
or fetching only the chunks and blocks a slice touches (``lazy=True``); see :func:`blosc2.open` and
… outlive a proxy Two more findings from the review, both of which reproduce. A chunk compressed against a codec dictionary keeps that dictionary between the block offsets and the blocks, so splicing rebuilt the chunk without it while leaving the flag that promises one: a lazy slice of an array written with use_dict=True raised "Error while getting the buffer". The header is what says so, so those chunks -- and variable-length-block ones, which do not use the zero-length stream the splice stands blocks in with, and any chunk without the extended header, whose offsets are not at byte 32 -- are now fetched whole, at the cost of the one header read that found out. Eviction only survived while the SChunk wrapper did. nspecialized is process-local, so a proxy that fetched a chunk, evicted it and closed without reading again left a persisted bitmap still claiming the chunk: the next run served the UNINIT zeros. The record lives in the cache's own vlmeta, so update_special now clears it there, which needs no proxy alive to notice. The blocks-per-chunk goes into the cache alongside the bitmap so the right bits can be found. Also the duplicated "or" the review spotted in installation.rst, which was mine from the previous round. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blosc2.full() writes a chunk that is its 32-byte header plus the value it repeats, stored at a real offset -- unlike a run of zeros, which the frame keeps in the offsets table itself and which wants_blocks already declines. chunk_layout read 32 + 4*nblocks bytes of such a chunk anyway, so it took the next chunk's bytes for block offsets: on a 40-byte chunk with 10 blocks that raised IndexError out of _block_extents, and with other data it would have spliced a chunk out of nonsense instead. The header says so, so the special-value kinds join the list chunk_layout declines, and a chunk whose own cbytes cannot cover a block-offsets section is refused whatever it claims to be. Reachable when BLOCK_MIN_CBYTES is lowered, which the tests do and which is a documented module constant; at the shipped megabyte a repeated-value chunk is 40 bytes and never gets that far. Found by reviewing the branch, not by a test failing. Also skip naming a chunk's blocks one at a time when a slice covers the chunk whole, which is most of a wide read: 10k blocks went from 8.3 ms to 0.4 ms per call, and a fully cached full read from 21 ms to 12 ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 17, 2026
Closed
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.
Using blocks instead of chunks when downloading partial datasets.