Skip to content

tantivy: count bundle range-read bytes by index file kind - #429

Merged
tonyalaribe merged 1 commit into
masterfrom
investigate/bundle-read-mix
Oct 25, 2026
Merged

tonyalaribe merged 1 commit into
masterfrom
investigate/bundle-read-mix

Conversation

@tonyalaribe

Copy link
Copy Markdown
Contributor

Observability only. After #426 a cold 7-day errors list on 6297304f reads ~1.1 GB of bundle ranges for 800 hits (~1.4 MB/hit vs ~255 KB/hit on the full path). Whether those bytes are fast-field columns, term data or the doc store decides the next fix (cache them on disk vs stop reading them).

Adds range_read_{fast,terms,store}_bytes to timefusion_stats (tantivy component); the remainder of range_read_bytes is other files. BundleSource::read_file is a provided method, so only the object-store source changes.

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

🤖 Generated with Claude Code

A cold 7-day errors list reads ~1.1 GB of bundle ranges for 800 hits (~1.4 MB per
hit, 5x the full path's ~255 KB). Whether that is fast-field columns, term data or
the doc store decides between caching and not reading it at all.
@monoscope-bot

monoscope-bot Bot commented Oct 25, 2026

Copy link
Copy Markdown

Monoscope production impact · Coverage unknown

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

Evidence window (UTC): 2026-10-18 15:33:07.387265537 UTC → 2026-10-25 15:33:07.387265537 UTC

Coverage: Open Monoscope for evidence and coverage details.

Review history and settings

@claude

claude Bot commented Oct 25, 2026

Copy link
Copy Markdown

Review of #429: bundle range-read bytes by index file kind.

The change is small and correct. I found no bugs, and I haven't run the tests myself.

  • Attribution: BundleDirectory::read passes the path through to read_file, and StoreSource buckets the span by extension. The extension mapping matches tantivy's segment components: .idx is postings, .pos is positions, .term is the term dictionary, .fast is fast fields, and .store is the doc store.
  • Fallthrough: .fieldnorm, .del and the like are not bucketed. They show up only as the remainder of range_read_bytes, as the PR description says.
  • Byte counts: the counters add up the compressed bytes of the zstd block spans fetched, not the decompressed bytes. That is the right measure for object-store IO. A short note on the field doc would help whoever reads these stats later.
  • Overhead: the extra cost is one extension match and one relaxed atomic add per span. That is negligible next to the S3 round trip.
  • Style: it fits CLAUDE.md. The default read_file leaves the Bytes source untouched, and the diff is minimal.

Suggestions, none blocking:

  1. Fieldnorm: if the remainder turns out large, add fieldnorm as its own counter. A small fieldnorm arm costs next to nothing.
  2. Double counting: read_file and read are two entry points on one source, so a future caller that overrides read_file and also calls self.read could double-count. The current code is fine. A one-line comment would help.
  3. Test: nothing asserts that the new counters move. A short test could open a bundle through StoreSource with a known layout, read a range from a .fast file, and check that range_read_fast_bytes equals the span and the three kind counters never sum to more than range_read_bytes. That would also cover the timefusion_stats wiring. If it's skipped because this is observability only, say so in the PR.

LGTM.

🤖 Generated with Claude Code

@tonyalaribe
tonyalaribe merged commit 2bede6a into master Oct 25, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant