Lazy remote POD5 reading over range requests (#83, phases 0–1) - #153
Open
jayhesselberth wants to merge 1 commit into
Open
Lazy remote POD5 reading over range requests (#83, phases 0–1)#153jayhesselberth wants to merge 1 commit into
jayhesselberth wants to merge 1 commit into
Conversation
… 0-1)
Read POD5 objects straight from S3/GCS/Azure/HTTPS instead of a local path,
fetching only the ranges a command actually needs. No POD5 format change --
this is reader-side I/O plumbing; the write path is untouched and remote
access is read-only.
escpod inspect summary s3://my-bucket/run1.pod5
escpod view https://example.org/data/run1.pod5
Gated behind a non-default `remote` Cargo feature at every layer
(escapepod-pod5 -> escapepod-signal -> escapepod-cli). A default build pulls
in no object_store/tokio/reqwest at all, and still recognises a URL so it can
refuse one with an explanation rather than reporting a missing path.
Phase 0 -- ByteSource. Reader holds an Arc<dyn ByteSource> instead of an Mmap
and asks for byte ranges, returned as refcounted Bytes. The local path stays
zero-copy: MmapSource slices a Bytes view over the mapping, so a range is a
refcount bump rather than a copy.
Rather than the crate-wide &[u8] -> Bytes migration the issue proposed (which
would have broken SignalExtractor<'a>), Reader caches each embedded table's
Bytes in a OnceLock and keeps handing out &[u8] borrowed from &self. That
gives a remote source the ownership it needs while leaving every existing
signature unchanged -- merge.rs, filter.rs, signal_extractor.rs, read_iter.rs
and the CLI all compiled untouched -- and it fetches each table at most once.
Footer parsing splits so it can run from the tail alone: footer_body_range()
turns the fixed 32-byte trailer plus the object size into the absolute range
holding the footer, and parse_footer_region() parses it. Reader::from_source
reads one 64 KiB tail, which in practice contains both, so a remote open is
two round trips.
Phase 1 -- RemoteSource over object_store, bridged into the synchronous Reader
by a single process-wide tokio runtime. Process-wide is load-bearing: rayon
workers call in concurrently, and a per-call runtime would be ruinous and
prone to nesting panics. read_ranges() is overridden to use get_ranges, which
coalesces adjacent ranges.
object_store refuses cleartext HTTP by default; when the user typed http://
themselves that refusal is noise, so allow_http is set for that scheme only.
https:// and the cloud schemes keep the strict default, and a cleartext S3
endpoint still requires AWS_ALLOW_HTTP=true. Remote errors now render their
full source chain -- object_store/reqwest/hyper each Display only their own
layer, so the real cause hid behind a bare "builder error".
Verification: scripts/test_remote_http.sh serves a POD5 over a Range-capable
HTTP server (python -m http.server ignores Range) and diffs local vs remote
output while counting bytes transferred. All three commands produce identical
output pulling 98 KB of a 1.77 MB file in 4 requests -- signature, footer
probe, run-info table, reads table. The signal table is never fetched.
Signal is still fetched a whole table at a time, so demux/resquiggle/repack/
merge remain a poor fit for a remote object; documented as such. Per-batch
lazy signal is issue phase 2.
Also fixed:
- Footer parsing rejects corrupt footer lengths instead of computing an
out-of-range slice offset. These bytes come from an untrusted file.
- Two escapepod-pod5 tests pointed at ../data/ instead of ../../data/ and had
been silently skipping their assertions; they now run.
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 #83 through phase 1. No POD5 format change — this is entirely
reader-side I/O plumbing; bytes on disk are untouched, and the write path is
not involved at all.
What this enables
summary,view, andinspectaccepts3://,gs://,az://, andhttp(s)://URLs wherever they accept a path. Opening transfers only the filetail and footer; the command then fetches just the reads table. Inspecting a
multi-GB object costs a few MB of range GETs instead of a full download.
It is behind a flag
remoteis a non-default Cargo feature, declared at each layer andforwarded down:
escapepod-pod5:remote = ["dep:object_store", "dep:tokio", "dep:url"]escapepod-signal:remote = ["escapepod-pod5/remote"]escapepod-cli:remote = ["signal", "escapepod-signal/remote"]A default build compiles no
object_store, notokio, noreqwest— andstill recognises a URL, so it refuses one with an explanation rather than
reporting a missing path:
Phase 0 —
ByteSource(no behaviour change)Readerno longer holds anMmap; it holds anArc<dyn ByteSource>and asksfor byte ranges, which come back as refcounted
bytes::Bytes.The local path stays zero-copy:
MmapSource::read_rangeslices aBytesviewover the mapping, so a range is a refcount bump, not a copy. There is a test
asserting the returned pointer is inside the mapping.
I deviated from the issue's plan in one way, for the better. The issue proposed
migrating
&[u8]→Bytescrate-wide and accepted a breaking API change(
SignalExtractor<'a>losing its lifetime). Instead theReadercaches eachembedded table's
Bytesin aOnceLockand keeps handing out&[u8]borrowedfrom
&self. That preserves the ownership story a remote source needs whileleaving every existing signature unchanged —
merge.rs,filter.rs,signal_extractor.rs,read_iter.rs, and the CLI compiled untouched. It alsomeans each table is fetched at most once, which is what a source with real
per-request cost wants anyway.
Footer parsing was split so it can run from the tail alone:
footer_body_range(trailer, file_len)— given the last 32 bytes and theobject size, returns the absolute range holding the footer.
parse_footer_region(region)— parses that range.Reader::from_sourcereads one 64 KiB tail, which in practice contains both,so a remote open is 2 round trips (head + tail).
Phase 1 —
RemoteSourceobject_storebehind a single process-widetokioruntime bridged withblock_on. Process-wide is load-bearing: rayon workers call into the readerconcurrently, so a per-call runtime would be both ruinous and prone to nesting
panics.
read_rangesis overridden to useget_ranges, which coalescesadjacent ranges.
Two things worth calling out:
object_storerefuseshttp://by default. When theuser typed
http://themselves that refusal is noise, soallow_httpis setfor that scheme only;
https://and the cloud schemes keep the strictdefault, and a cleartext S3 endpoint still requires
AWS_ALLOW_HTTP=true.object_store→reqwest→hypereachDisplayonlytheir own layer, so the real cause hides two links down — the failure that
led to the
allow_httpfix surfaced as a barebuilder error. Remote errorsnow render the full source chain.
is_remote_urladvertises is oneparse_url_optscan actually route caughtthat
az://container/pathcarries no account, so store construction needsAZURE_STORAGE_ACCOUNT_NAME— documented, and the test now supplies theminimum config per backend rather than assuming all seven build bare.
Verification
cargo nextest run --workspace: 396 passed.cargo test --doc --workspace:19 passed.
cargo nextest run -p escapepod-pod5 --features remotegreen.cargo clippy --workspace --all-targetsand the same with--features escapepod-cli/remote: zero warnings.cargo fmt --checkclean.End-to-end HTTP parity, reproducible via
scripts/test_remote_http.sh(added). It serves a POD5 over a Range-capable HTTP server —
python -m http.servercan't be used, it ignoresRange— and diffs local vs remoteoutput while counting bytes actually transferred:
The four ranges are the leading signature (8 B), the 64 KiB footer probe, the
run-info table, and the reads table. The signal table is never fetched —
which is the whole claim.
Also fixed along the way
an out-of-range slice offset. A negative length, or one placing the footer
magic before the file's leading signature, is a clean
InvalidFootererror.These bytes come straight from an untrusted file.
escapepod-pod5tests pointed at../data/…instead of../../data/…and had been silently skipping their assertions for as long as they've
existed. They now actually run (visible as
Parsed 2 batches from 1698.31 KB signal tablein test output).Not in scope (issue phases 2–3)
Signal is still fetched a whole table at a time, so
demux,resquiggle,repack,merge, andfilterwould pull essentially the entire object overthe network. Documented as such, with the advice to download first. Per-batch
lazy signal is phase 2. Remote access is read-only; there is no remote write
path.