test(python): add data overlay benchmark suite#7544
Conversation
1156dd2 to
687507f
Compare
687507f to
92734fa
Compare
d1e3be6 to
d1fa12b
Compare
92734fa to
9a09276
Compare
91b3987 to
ecfb29c
Compare
6d52845 to
968d024
Compare
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
A scalar or vector index built before an overlay does not reflect the overlay's values, so its entries for overlay-covered cells may be stale. Queries must stay correct while overlays remain. For each index a query relies on, compute the set of fragments carrying an overlay that was committed after the index (`committed_version > index.dataset_version`) and that touches a field the index covers. The check is field-aware (an overlay on an unrelated field excludes nothing) and version-gated (an overlay already incorporated by the index is ignored), via the new `overlay_exclusion_offsets` helper. Such fragments are dropped from the index's covered set so they fall to the flat path, which re-evaluates them against their current (overlay-merged) values: - Scalar (`FilteredReadExec`): stale fragments are removed from the `EvaluatedIndex` covered set, so the full filter is re-applied to them. - Vector: stale fragments are removed from the index segments' coverage bitmaps (so the ANN prefilter blocks their stale rows) and added to the flat-KNN fallback so their current vectors are re-scored into the top-k. This drops stale index hits and surfaces new matches the index never saw. Granularity is per fragment; row-level exclusion is a future optimization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two optimizations to reduce per-query overhead of the overlay-stale-index check: 1. `mask_overlay_stale_segments` now returns `Option<Vec<IndexMetadata>>` instead of always cloning the segment list. `None` means "use the original slice unchanged" — no allocation on the fast path (no overlays anywhere or no stale fragments found). Only the path that actually filters stale rows from coverage bitmaps clones the segments. 2. `collect_stale_overlay_frags` adds a cheap version pre-check before calling `overlay_exclusion_offsets`: if every overlay on a fragment predates the index (`committed_version <= segment.dataset_version`), the fragment is skipped without loading any coverage bitmaps. Also adds an `#[ignore]` benchmark (`bench_index_query_overlay_overhead`) covering BTree, FTS, and vector ANN queries at 0/4/16 overlay layers so the overhead can be measured directly. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Address understandability gaps flagged in self-review: - `overlay_stale_index_frags`: explain that `ScalarIndexExpr` has no built-in visitor (motivating the manual stack traversal), and note that `load_named_scalar_segments` hits the cached index manifest so the per-leaf cost is bounded. - `partition_frags_by_coverage`: explain why stale fragments do not need to be threaded downstream as in the vector path — the scalar flat path already handles them via `missing_frags`, making explicit re-scoring unnecessary. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Two additions from polish-pr self-review: 1. `test_overlay_stale_with_compound_index_expression`: exercises the ScalarIndexExpr tree-walk in `overlay_stale_index_frags` with an AND predicate over two independently-indexed columns (age AND id). Closes the coverage gap where no e2e test covered multi-leaf index expressions. 2. `TODO` comment in the `fts` method noting that FTS does not yet mask overlay files — a stale FTS index entry for an overlaid text field would survive until compaction. This is out of scope here but documents the known gap and records what the fix would require. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Replace the 1,500-row in-memory benchmark with a realistic 1M-row disk-based harness (10 fragments × 100k rows, 32-dim vectors). Two scenarios that exercise the actual correctness overhead: Scenario A (BTree): overlay on `age` (the indexed field). Fragment 0 falls to a 100k-row flat scan instead of an O(log n) index lookup. Measures `cold_frag0_ms` (stale fragment) vs `warm_frag1_ms` (index-served) at 0/1/4/16 overlay layers to isolate flat-scan cost from index-lookup baseline. Scenario B (Vector ANN): overlay on `vec` (the indexed field). The field-aware check confirms the BTree age overlays leave the vector index clean. One vec overlay on fragment 0 forces brute-force KNN across all 100k rows of that fragment (O(100k × 32) extra distance computations). Measures `ann_ms` at 0 vs 1 vec overlay. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… stores
commit_overlay wrote overlay files via Path::from("data/{name}"). For file://
stores, to_local_path() just prepends '/', making this resolve to /data/{name}
(root filesystem, Permission Denied). For memory:// stores it happened to work
because that branch doesn't call to_local_path at all.
Fix: use dataset.base.clone().join("data").join(filename) so that for file://
stores the path includes the dataset's base directory components, which when
prepended with '/' give the correct absolute path. For memory:// stores base
is the empty path, so join("data").join(name) produces the same string as before.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Previously, when any row in a fragment had a stale vector overlay, the entire fragment (up to 100k rows) was removed from ANN index coverage and re-scored on the flat path — adding ~1.9ms overhead per stale fragment in benchmarks. This changes the vector ANN path to row-level precision: - Stale row addresses are computed per-fragment (not whole-fragment) - A RowAddrMask::BlockList for stale rows is passed to DatasetPreFilter.overlay_block, blocking them from ANN results via the existing prefilter mechanism - Only the specific stale row addresses are re-scored via a targeted TakeExec + flat KNN path, not the whole fragment - Non-stale rows in the same fragment remain in ANN coverage For a fragment with 100k rows and 1 stale row, overhead drops from O(100k) flat KNN to O(1) targeted take. New infrastructure: - DatasetPreFilter::with_overlay_block for synchronous row-level blocks - ANNIvfSubIndexExec stores and applies overlay_block at execute time - new_knn_exec accepts Option<RowAddrMask> for overlay blocking - collect_overlay_stale_rows_for_segment: per-row stale computation - Scanner::mask_overlay_stale_rows: replaces mask_overlay_stale_segments Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
FTS queries could return stale hits when an overlay committed after the index build covers a fragment the FTS index also covers. The inverted index entries for those rows still reflect the pre-overlay values. Fix: in `plan_match_query`, compute which FTS segments touch stale fragments (overlay committed_version > segment.dataset_version on the indexed column) via `fts_stale_frags_and_fresh_segments`. Those segments are excluded from `MatchQueryExec` (using `new_with_segments` with the fresh subset) and all fragments they covered fall to the flat `FlatMatchQueryExec` path, which reads current overlay-merged values. The fast path (no overlays) is O(n fragments) with no segment loading. Adds two tests in overlay_index_masking: - test_fts_overlay_stale_drop_and_new_match: stale term no longer returned, new term found via flat path - test_fts_overlay_unrelated_field_not_excluded: overlay on a non-FTS field does not affect FTS coverage Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Previously, any stale overlay on a fragment caused the entire fragment to fall to the flat scan path (missing_frags), scanning up to 100k rows to re-evaluate the predicate for a single stale row. Now the BTree path matches the vector ANN approach: - partition_frags_by_coverage replaces the fragment-level overlay_stale_index_frags with overlay_stale_index_rows (row-level). Stale-but-indexed fragments stay in relevant_frags; only their per-row stale offsets are returned as a HashMap<u32, RoaringBitmap>. - MaterializeIndexExec gains overlay_block: Option<RowAddrMask> + with_overlay_block builder. The block mask is ANDed into the candidate mask before row_ids_for_mask, so stale index entries never reach downstream operators. - scalar_indexed_scan builds the block list from the stale row map and applies it to MaterializeIndexExec. A new stale-Take union path (OneShotExec(stale_row_ids) -> TakeExec -> LanceFilterExec(full_filter) -> project) re-evaluates only the stale rows against their current overlay-merged values and unions with the indexed path. Non-stale rows in a fragment with a stale overlay remain on the indexed path; only the specific stale rows pay the take cost. Adds test_btree_overlay_row_level_precision: overlays one row in a fragment so that two rows in the same fragment share the queried value. Verifies the non-stale row is returned (from the index) alongside the stale row (from the Take path). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…, cleanups Addresses review of the overlay index-masking work. - Scalar (V2 FilteredReadExec): previously the default v2 path computed per-row stale offsets, then collapsed them to fragment ids and dropped whole fragments to the flat path — only the legacy v1 scalar_indexed_scan path was row-level, and v1 cannot carry overlays. Block just the stale rows from the index result (via EvaluatedIndex::without_rows / FilteredReadOptions::overlay_block) and re-evaluate them with a targeted Take + full-filter path unioned with the indexed read, so v2 scalar masking is O(stale_rows), not O(fragment_size). - FTS phrase queries: plan_phrase_query loaded all segments unmasked, so a stale overlay could return stale phrase hits. Exclude segments covering stale fragments. Phrase has no flat re-evaluation path, so — like unindexed fragments, which phrase already does not search — overlaid fragments are dropped from the phrase result (matches surface after compaction); when every segment is excluded the query returns empty rather than erroring. Correct the stale fts() TODO that claimed FTS was unmasked. - Perf: stale-collection now iterates the (rare) overlaid fragments and probes segment coverage, O(overlaid_frags) instead of O(indexed_frags); the vector helper is scoped to the query's target fragments. - Cleanups: extract shared stale-rows block-mask and row-id-exec helpers (used by the scalar, vector, and v2 paths), encode row addresses via RowAddress::new_from_parts, relocate the collect_* helpers next to overlay_exclusion_offsets in dataset::overlay, rename mask_overlay_stale_rows -> overlay_stale_vector_rows, import HashMap, use RoaringBitmap over HashSet<u32>, and fix doc drift referencing overlay_stale_index_frags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scanner.rs had grown past 14.5k lines. Relocate the self-contained overlay_index_masking test module (~1000 lines) into the established dataset/tests/ directory as dataset_overlay_index_masking.rs. The tests use only crate-public APIs, so no visibility changes were needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Index results are in the row-id domain, and a physical row address equals its row id only when the dataset does not use stable row ids. The overlay stale-row masking expressed rows as physical addresses everywhere, so under stable row ids the block never matched the index results (stale hits leaked) and the re-scored rows were read wrong. Translate the stale addresses to the row-id domain (via translate_addr_treemap_to_row_ids) at every masking point: the scalar V2 take (FilteredReadExec), the scalar V1 block (MaterializeIndexExec), and the vector ANN prefilter (DatasetPreFilter). Parametrized scalar and vector tests over enable_stable_row_ids, overlaying a non-first fragment where address != row id. Also addresses PR review: - Use an address allow list (not a `_rowid` OneShotExec + TakeExec) to identify the stale re-eval rows; `_rowid` is a logical id under stable row ids, so the old path took the wrong rows. Consolidated into `stale_rows_take`. - Treat a missing `fragment_bitmap` as covering all fragments in the overlay collect helpers and in FTS segment classification, matching DatasetPreFilter, so stale rows can't slip through unmasked on legacy bitmap-less indices. - Gate the exact-index prefilter shortcut on there being no stale rows, so a stale selection vector can't reach ANN/FTS. - Replace ANNIvfSubIndexExec::try_new_with_overlay with a with_overlay_block builder. - Extract the shared version-gate + exclusion-offsets helper in overlay.rs. - Replace bare unwraps with `?`/expect_ok, debug_assert the block-list invariant in without_rows, and move test-only imports to the tests module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add regression coverage for overlay index masking under fast_search and for the legacy missing-fragment_bitmap branch: - test_btree_overlay_masked_under_fast_search: the scalar drop-stale block and stale-Take re-eval both run regardless of fast_search. - test_vector_overlay_stale_dropped_under_fast_search: the ANN prefilter block drops the stale hit under fast_search while the flat re-score is skipped (recall tradeoff); guards against moving overlay_block inside the !fast_search guard. - test_collect_frags_missing_bitmap_covers_all / _rows: a None fragment_bitmap (index predating bitmap tracking) is treated as covering every fragment. Self-review cleanups: - Move the misplaced doc block onto collect_overlay_stale_frags (renamed from collect_stale_overlay_frags for consistency with the row-level collector); the private helper keeps its // comment. - Restore the "can't pushdown limit/offset" and "re-ordering anyways" comments dropped during the knn_combined rewrite. Extract create_vector_overlay_dataset/vector_query_ids helpers (shared with test_vector_index_rescore_on_overlay) and an ids_matching_opts fast_search variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Overlay-aware index masking adds fields to widely-shared read-path types (e.g. DatasetPreFilter, ANNIvfSubIndexExec), which pushes the layout depth of several scan/take futures past rustc's default query-recursion limit (128) on 1.97. Raise the limit to 256 in the lance lib and in the one bench crate (mem_wal_point_lookup_bench) that exercises the deepened path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-review pass to reduce diff size without changing behavior: - Tests: merge the two FTS id-collecting helpers into one `fts_ids(query)` with thin term/phrase delegators, and extract `ids_from_batches` (the id-column -> Vec<i32> pattern was copied in 4 helpers). Hoist repeated in-fn `use` imports (StringArray, InvertedIndexParams, MetricType, VectorIndexParams, TryStreamExt) to the module top and drop a redundant IndexType re-import. - scanner: extract `overlaid_fragments` (the overlaid-fragment map was built 3x) and `stale_rows_in_id_domain` (shared tree-build + stable-row-id translation between the block-mask and take paths), consolidating the address-vs-rowid rationale into one place. - Trim the doc comments on the `with_overlay_block` setters to point at the field doc instead of repeating it, and shorten the `new_knn_exec` doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…st nits Address review feedback: - Replace the `#![recursion_limit = "256"]` attributes (lance lib + the point- lookup bench) with `Box::pin` on the two deep futures that overlay-aware index masking tipped past rustc's default limit: the `remap_index` call in `remap_indices` and the bench's `plan_lookup`. Boxing caps the future's layout depth locally rather than papering over it crate-wide. - Drop `max_rows_per_group` from the test datasets — row groups don't exist past Lance 1.0. - Remove an unnecessary comment on `create_base_dataset_with`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…limit overflow
Overlay-aware index masking deepens create_plan's async layout. Two inline
callers push it past rustc's default recursion limit ("queries overflow the
depth limit!"), surfacing in coverage-instrumented CI builds:
- Scanner::explain_plan (fm_contains_bench)
- the mem_wal point-lookup scan builder (mem_wal_point_lookup_bench)
Box the future at each site, the same way try_into_stream and count_rows
already box their plan futures. Boxing per-site (rather than inside create_plan)
keeps create_plan's future type unchanged, avoiding an E0275 Send-bound
trait-solver overflow that centralized boxing triggered in downstream crates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a Python benchmark suite for data overlay files in python/ci_benchmarks/, covering three areas: - manifest size growth vs. number of overlays and coverage, - bytes written updating 1% of a column via an overlay vs. update / merge_insert / full rewrite, across narrow and wide rows, - take and scan time + cold read IO as overlay layers, coverage fraction, fragmentation, and data type vary. Shared dataset/overlay construction and measurement helpers live in ci_benchmarks/overlays.py and build fixtures through the public lance.LanceOperation.DataOverlay binding. The write benchmark verifies each approach actually performs the update and splits persisted bytes into data vs. metadata, since the rewrite mechanisms are delete-and- reinsert (whole rows, all columns) while an overlay writes only the changed column. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The overlay read benchmark's take/scan scaling sweep only covered a 4-byte int32 value column, where the base read is trivial and per-layer merge cost dominates. ML overlays typically target wide vector columns, so add test_overlay_read_wide: the same overlays x coverage-pattern grid on a 3072-d float32 embedding (12 KiB/row). Thread an embedding_dim through the overlay helpers so the value width is configurable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… API `DataOverlayFile` takes a single `offsets` param, not `shared_offsets`, and the release-build opt-in env var is `LANCE_ENABLE_UNSTABLE_DATA_OVERLAY_FILES` (not `LANCE_ENABLE_DATA_OVERLAY_FILES`). Both diverged from the benchmark helper after PR #7540 landed the real Python bindings; commit()'s returned handle skipped flag validation so this only surfaced on a fresh dataset open.
40a4a5c to
c3837ce
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Benchmark resultsRebased onto latest Setup: 1M rows, local disk (NVMe), Manifest growth (
|
| overlays | 1% contiguous | 1% stride | 10% stride |
|---|---|---|---|
| 1 | 16.4 KB/ea | 40.2 KB/ea | 253 KB/ea |
| 4 | 10.3 KB/ea | 25.2 KB/ea | 158 KB/ea |
| 16 | 8.8 KB/ea | 21.4 KB/ea | 134 KB/ea |
| 64 | 8.4 KB/ea | 20.5 KB/ea | 128 KB/ea |
(64 layers @ 10% stride → ~8.2 MB manifest total.)
Write cost (test_overlay_write.py)
Updating 1% of rows via an overlay vs. the existing rewrite-based mechanisms. narrow = id + val; wide = id + val + 64-d float32 payload:
| approach | narrow: read / persisted | wide: read / persisted |
|---|---|---|
| update | 6.2 MB / 100 KB | 8.7 MB / 2.66 MB |
| merge_insert | 2.4 MB / 101 KB | 2.4 MB / 2.66 MB |
| merge_insert (indexed) | 12.7 MB / 100 KB | 15.3 MB / 2.66 MB |
| overlay | 143 B / 105 KB | 143 B / 105 KB |
| full_rewrite | 6.2 MB / 6.17 MB | 262 MB / 262 MB |
The rewrite-based approaches delete-and-reinsert whole rows, so their cost scales with row width. An overlay only ever touches the changed column: read bytes stay ~0 and persisted bytes stay constant (~105 KB, dominated by the coverage-bitmap + manifest overhead) regardless of how wide the row is — the advantage over update/merge_insert grows directly with row width, and vs. full_rewrite it's already ~60-2500x smaller at this width.
Read scaling (test_overlay_read.py)
1M-row int32 scan/take, strided coverage, v2.0 format:
| overlays | take (mean) | scan (mean) |
|---|---|---|
| 0 | ~0.25 ms | ~1.7 ms |
| 4 | ~0.7 ms | ~7-8 ms |
| 16 | ~1.2 ms | ~40 ms |
- take stays roughly flat (rank pushdown means only the requested rows are touched).
- scan degrades close to linearly with overlay-layer count (~24x at 16 layers) — this is the existing
route_overlayscost profile (offset-major bitmap probing), not something this PR changes. It's the strongest argument for overlay compaction. - Wide embeddings (3072-d): scan is ~600-870 ms regardless of overlay count, since base I/O volume dominates over overlay-routing cost at that width.
cc for the demo this week: the standout numbers are the overlay write cost (143 B read, constant persisted bytes independent of row width) and the scan-cost-vs-layer-count curve as the motivating case for compaction.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Adds a Python benchmark suite for data overlay files in
python/ci_benchmarks/, stacked on #7540 (which exposes theDataOverlaycommit binding these benchmarks use to build fixtures).The benchmarks generate their own base datasets and overlays into a temp dir, so no pre-generation step is needed. Three areas:
test_overlay_manifest.py— manifest growth vs. number of overlays and coverage size/pattern.test_overlay_write.py— bytes written updating 1% of a column via an overlay vs.update/merge_insert/ full rewrite.test_overlay_read.py— take and scan wall time (pytest-benchmark) plus cold read IO (io_stats_incremental) as overlay layers, coverage fraction, fragmentation (contiguous vs. strided), and data type (int32 vs. fixed-size-list embedding) vary, across v2.0 and v2.1.Shared dataset/overlay construction and measurement helpers (
/proc/self/io, on-disk size, manifest size) live inci_benchmarks/overlays.py.Notes:
update-via-overlay path does not exist yet, so the write benchmark's overlay arm is hand-rolled and writes fresh values without reading the base column first; the headline metric is bytes written, latency secondary.🤖 Generated with Claude Code