Lazy roaring flags bitmap and bool index counts - #9749
Conversation
Opening a read-only segment scanned every flags file end to end:
`ReadOnlyRoaringFlags::open` materialized the whole RoaringBitmap via
`iter_ones()`. Every payload field carries a null index, so this was paid
per field per segment, for bitmaps most queries never touch.
Make the bitmap a `OnceLock`, filled by a scan on first access. Open now
reads only the tiny status file. `ReadOnlyBoolIndex`'s three eager count
fields collapse into one lazily-derived, cached `BoolCounts`; its
`live_reload` refreshes them in place when present and leaves them unset
otherwise, so reloading an index nothing queries stays scan-free.
Propagate the resulting `OperationResult` through `RoaringFlagsRead`,
`PayloadFieldIndexRead::count_indexed_points`, `FieldIndexRead`,
`PayloadIndexRead::{indexed_points, get_telemetry_data}`, `build_info` /
`build_telemetry` and `SegmentEntry::{info, get_telemetry_data}`, out
into shard, edge and collection.
`ram_usage_bytes` stays infallible: an unmaterialized bitmap holds no
RAM, so it reports 0 via the new `bitmap_if_materialized`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR changes segment, shard, and field-index read APIs to return Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
`EdgeShardRead::info` now returns `OperationResult<ShardInfo>`. The examples live in their own workspace (lib/edge/publish), so the main `cargo check --workspace` never saw them. Every call site sits in `fn main() -> Result<(), Box<dyn Error>>`, so propagate with `?`. `bm25-search` compiled either way but would have printed the `Result` rather than the `ShardInfo`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coszio
left a comment
There was a problem hiding this comment.
Looks good, it was a crazy amount of changes to consider the OnceLocks.
It would be nice to propagate the Populate option through preopen and open. We can do it outside of this PR.
yes, I think it should be part of query-specific load profile |
* [AI] make ReadOnlyRoaringFlags bitmap and bool index counts lazy
Opening a read-only segment scanned every flags file end to end:
`ReadOnlyRoaringFlags::open` materialized the whole RoaringBitmap via
`iter_ones()`. Every payload field carries a null index, so this was paid
per field per segment, for bitmaps most queries never touch.
Make the bitmap a `OnceLock`, filled by a scan on first access. Open now
reads only the tiny status file. `ReadOnlyBoolIndex`'s three eager count
fields collapse into one lazily-derived, cached `BoolCounts`; its
`live_reload` refreshes them in place when present and leaves them unset
otherwise, so reloading an index nothing queries stays scan-free.
Propagate the resulting `OperationResult` through `RoaringFlagsRead`,
`PayloadFieldIndexRead::count_indexed_points`, `FieldIndexRead`,
`PayloadIndexRead::{indexed_points, get_telemetry_data}`, `build_info` /
`build_telemetry` and `SegmentEntry::{info, get_telemetry_data}`, out
into shard, edge and collection.
`ram_usage_bytes` stays infallible: an unmaterialized bitmap holds no
RAM, so it reports 0 via the new `bitmap_if_materialized`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [AI] correct `preopen` comment: `open` no longer scans the flags file
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [AI] fix edge examples for fallible `info()`
`EdgeShardRead::info` now returns `OperationResult<ShardInfo>`. The
examples live in their own workspace (lib/edge/publish), so the main
`cargo check --workspace` never saw them.
Every call site sits in `fn main() -> Result<(), Box<dyn Error>>`, so
propagate with `?`. `bm25-search` compiled either way but would have
printed the `Result` rather than the `ShardInfo`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Opening a read-only segment scanned every flags file end to end.
ReadOnlyRoaringFlags::openmaterialized the wholeRoaringBitmapviaiter_ones(), and since every payload field carries a null index, that cost was paid per field per segment — for bitmaps most queries never touch. It defeats the point of prefetching only the bytes a query needs.Note the counts were not the culprit:
ReadOnlyNullIndex::opencomputes none at all (indexed_points_count()ishas_values_flags().len(), straight from the tiny status file). The eager scan was the bitmap itself, so that is where the laziness had to go.ReadOnlyRoaringFlags::bitmapis now aOnceLock<RoaringBitmap>, filled by a scan on first access.opentouches only the status file. (OnceLockbecause indexes are queried through&selfacross threads;get_or_try_initis still unstable, so a race may build twice and drop the loser.)ReadOnlyBoolIndex's three eager count fields collapse into one lazily-derived, cachedBoolCounts. Itslive_reloadrefreshes them in place when they are already present, and leaves them unset otherwise — so reloading an index nothing queries stays scan-free. The refresh is free when it runs: counts can only exist if deriving them already materialized both bitmaps, whichlive_reloadpatches in place.ram_usage_bytesstays infallible: an unmaterialized bitmap holds no RAM, so it reports 0 via the newbitmap_if_materialized.Propagation
Materializing on demand can fail, so
Resultpropagates fromRoaringFlagsRead::{get_bitmap, get, iter_trues, iter_falses, count_trues, count_falses}throughPayloadFieldIndexRead::count_indexed_points,FieldIndexRead::{get_telemetry_data, values_count, values_is_empty, value_retriever},PayloadIndexRead::{indexed_points, get_telemetry_data},build_info/build_telemetry, andSegmentEntry::{info, get_telemetry_data}— out intoshard,edge(including the publicEdgeShardRead::info) andcollection. Most consumers were already inOperationResultcontexts.bool_index/read_ops.rs::value_retrievermust return an infallible per-point closure, so both bitmaps are resolved once at construction rather than per point.Two collection sites read
info()for size fields only; they now call the infalliblesize_info(). That is strictly better:/telemetryno longer forces every bool bitmap into memory.Behavior change worth reviewing
ProxySegment::size_info()used to delegate toinfo(), so it uniquely returned a populatedindex_schema. To keep it infallible the proxy adjustments moved intoadjusted_info(), fed by the wrapped segment'ssize_info(), which by contract leavesindex_schemaempty (read_view/info.rs:27). The proxy now matches plainSegment::size_info(). No caller readsindex_schemaoffsize_info(), but this is a semantic change, not a pure refactor.Test plan
The existing
live_reload_matches_fresh_opentests never read the index before reloading, so onceopenstopped materializing, they stopped covering the in-place delta path entirely — I disabledlive_reload's deletes and appends and both still passed. Each is now split into_materialized/_lazyvariants; the sabotage fails the_materializedpair.Each half of the new
live_reloadcontract is pinned independently, verified by breaking it:refresh_countsa no-op (stale counts survive) failslive_reload_matches_fresh_open_materializedlive_reload_matches_fresh_open_lazyNew
open_does_not_materialize_bitmapspins the laziness itself — nothing else in the module would notice an eageropen, since every other test reads the index.cargo test -p segment --lib— 867 passed, 0 failedcargo test -p collection --lib— 244 passed, 0 failedcargo test -p shard --lib/-p edge --lib— 35 / 86 passedcargo clippy --workspace --all-targets— clean🤖 Generated with Claude Code