[tonyalaribe]

Follow-up to #429. Exact-range disk-cache keys only hit when a read asks for the same run of blocks that filled the cache. A restart reproduces that (verified: 30d first load read 1,022 MB from disk), but a partly evicted block cache asks for different runs: a busy 2-hour-old process refetched 828 MB of a 7-day errors list from S3 that the disk tier already held (10.1 s first load).

StoreSource now learns the bundle length when the directory is built and serves reads from aligned 1 MiB chunks: chunk lookups run concurrently, each contiguous run of missing chunks is one GET, fetched chunks are inserted disk-only, and the range is assembled by slice copies.

Test: an_overlapping_range_is_served_from_disk_chunks (a range never read whole is served with zero object-store reads; each attempt uses a new range so retries cannot serve themselves); red with range-dependent keys.

Local signoff: make ci-signoff fmt/test/pg-smoke/e2e/clippy all pass (attested).

🤖 Generated with Claude Code

[monoscope-bot]

Monoscope production impact · Worth checking

Project 87576849-4941-49d3-a15d-680fef88a1a8 · reviewed d6941781cdbf170feed17e69e738d34fd44e0206 · advisory

Evidence window (UTC): 2026-10-18 23:34:36.258669085 UTC → 2026-10-25 23:34:36.258669085 UTC

src/tantivy/search.rs:956 (Head)

Production details are available in Monoscope.

telemetry evidence

Coverage: Open Monoscope for evidence and coverage details.

Review history and settings

[claude]

Review

Overall: a clean, focused change. The aligned-chunk design fixes the exact-range key misses, and the test is well thought out (fresh range per attempt, so retries can't serve themselves). Notes below, roughly by importance.

Read amplification on cold misses. Any miss now fetches at least one whole 1 MiB chunk. That includes the many tiny reads tantivy makes (term dict and postings fragments of a few KB) on a first-ever read of a region. range_read_bytes will therefore rise on a cold cache and no longer reflect bytes the caller asked for. This is probably the right trade given the 828 MB refetch, but please say so in the PR. Consider a metric or comment that separates "requested" from "fetched" bytes so the stats stay interpretable.

Short-chunk poisoning if len is wrong. read_chunked caches span.slice(..) as the chunk for the key. If len is ever larger than the real object (a stale head or a rewritten blob at the same path), a truncated final chunk is cached under a key that implies a full one, and later reads silently return short data. The .min(bytes.len()) clamp in assembly hides this instead of surfacing it. Cheap hardening: check that the fetched span length equals the requested length and return an io::Error if not. This also relies on bundle paths being immutable (the put_immutable contract), which is worth a one-line comment.

Edge cases when range.end > len. ((hi + 1) * DISK_CHUNK).min(len) can fall at or below lo * DISK_CHUNK, which gives an empty or inverted range. A debug_assert!(range.end <= len), or falling back to fetch in that case, would be safer than relying on callers.

Minor

• Assembly always copies into a new Vec. When the range falls inside a single chunk, which is the common case for small reads, bytes.slice(start..end) is zero-copy and avoids an allocation per read.
• range_disk_hits now counts per chunk, where it counted per read before. If dashboards compare it against range_reads, the units differ. Worth noting.
• The old tantivy-bundle: keys are orphaned in the disk tier until evicted. That's harmless, but it temporarily wastes capacity right after deploy.
• let _ = source.len.set(...) followed by with_cache is a little awkward. If BundleDirectory::new could take the length-setting step, the OnceLock plumbing would be less visible. Not blocking.
• Only put_immutable insertions are covered by the test. A case for a read that straddles a cached chunk and a missing one would cover the multi-run chunk_by logic, where an off-by-one would be easy to introduce. Adding it to the same test with a case table is cheap.

Nothing blocking from a security standpoint: keys are derived from internal paths and there is no user-controlled input. Looks good to merge once the short-chunk check is considered.