Skip to content

feat(engine): expose ann diagnostics and budget native writer buffers - #1086

Merged
aaltshuler merged 3 commits into
ModernRelay:mainfrom
aaltshuler:codex/lance-ann-writer-budgets
Oct 20, 2026
Merged

aaltshuler merged 3 commits into
ModernRelay:mainfrom
aaltshuler:codex/lance-ann-writer-budgets

Conversation

@aaltshuler

@aaltshuler aaltshuler commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What & why

Expose Lance 13 ANN diagnostics in the existing query inspection report/profile, and use the captured write_max_bytes allowance for native buffering on strict inserts and overwrite writes. This adds no CLI flags, settings, dependencies or storage format.

  • Optional ann_metrics rows identify the scan, overfetch rung and probe attempt. Six curated counters and twelve cumulative stage timings retain missing metrics as absent. Timings can overlap and never drive search or replay decisions. partitions_ranked sums the IVF partitions across opened index deltas; it is not a recall measure.
  • GQT validates metric names/types, scan/rung/attempt joins and same-run profile preservation. Existing nearest, filtered, overfetch and two-vector RRF cases require diagnostics present. Timing values stay outside DST replay observations; controlled native-counter and synthetic adapter tests remain in Rust.
  • Strict/proven inserts and nonempty overwrite paths pass the captured WriteBudget to FileWriterOptions.data_cache_bytes, with copying enabled. Native merge-insert upserts/updates retain Lance's defaults because its builder has no writer-options setter. Full arrays, nested encoders and concurrent writers can exceed this per-writer flush threshold. At the default 32 MiB, the per-column threshold can exceed Lance's native 8 MiB for narrow tables. Writes retain the existing dataset's V2_2 format.

Backing issue / RFC

Maintainer-requested adoption follow-up to merged #1072. Existing detached publication and deterministic simulation contracts remain unchanged (RFC 0067, RFC 0037).

This PR is rebased onto current main, including the Lance 13 migration and CLI cleanup. Its diff contains only this adoption slice. Main's GQT pipeline assertions and separate load-explain guide are preserved.

Validation

  • Clean focused baseline before the rebase: 124 engine/report/staged-write and GQT parser/report tests passed, zero failed or ignored.
  • Independent reviews of ANN/GQT and writer budgeting against Lance 13 source found no implementation blocker. A post-rebase range review confirmed that the conflict resolutions preserve both changes.
  • Rebased local owners: 830 passed, zero failed, one existing heavy test ignored: 521 engine units, 213 GQT-core units, 28 search, 63 writes and five scrubbed-replay tests. Command: RUST_MIN_STACK=16777216 cargo test --locked -p omnigraph-engine -p omnigraph-gqt-core --lib --test search --test writes --test engine_v2_scrubbed_replay -- --test-threads=4.
  • Documentation, formatting, spelling, AGENTS links/environment rules, dependency-source checks and git diff --check pass. A committed 0.14 release-note preview also composes successfully.
  • Fresh CI passed workspace/failpoint tests, Clippy, S3 default/failpoints, Azurite, AWS server and storage-upgrade compatibility.
  • Fresh GQT passed all modes, including all 409 corpus cases with DST.
  • Fresh DST passed, explicitly including dst_bench_cost_count_golden; no regeneration was needed after rebasing.

Storage-cost evidence

The staged overwrite owner reads native file metadata: the same three rows across two nonempty batches produce two hot-column pages at 128 KiB and one at the default 32 MiB, with one cold-column page, complete row/schema roundtrip and unchanged HEAD before commit. The strict/proven insert conflict owner also uses a small captured budget.

Only six new golden rows are added; every existing row is unchanged. Each label measures one public overwrite on a reopened handle, excluding setup and verification:

Label Reads (l.get) Listings (l.list) Writes (l.put)
OverwriteWriter128KiB 13 1 6
OverwriteWriter32MiB 13 1 6

Both settings read the existing state and stage/publish the same replacement. The one-batch public fixture preserves request counts at both thresholds; the staged owner separately proves the multi-batch page-layout difference. These are storage-call counts, not peak-memory or throughput measurements.

@cursor

cursor Bot commented Oct 19, 2026

Copy link
Copy Markdown

Verdict

Comment. The two follow-up commits match Lance 13.0.0 for the counters they publish and for the writer option they can set. Do not merge bb2857ffdf394a34a6608cf6c9760dbdc0c15469 on the current pull-request base. Land #1072, rebase this branch onto main at 9e2425d26a03b1b5628030a359ee51f096d3ca1c, and require a green GQT (dst) run.

What this change does

This branch contains the open Lance 13.0.0 upgrade (#1072, through 91ecceefb2457b0cdcaf9bc6dc2a88f5bce69061) and two later commits. Those two commits add nearest-scan diagnostics and pass the existing write_max_bytes setting into Lance file writers on two write paths.

Diagnostics are observations. AnnAttemptMetrics::from_summary copies a fixed set of counts and stage times from the scan summary into the execution report. Executed::profile prints those rows with node ann_metrics. The probe ladder still decides from partitions_searched, partitions_ranked, and the probe row. Replay does not compare the new rows.

The writer change sets FileWriterOptions.data_cache_bytes from WriteBudget on absence-proven strict inserts and on nonempty staged overwrites. keep_original_array is Some(false). max_page_bytes stays unset. Upserts and updates stay on merge-insert.

Blocking

Recorded GQT (dst) on this head is red. The failure belongs to the pull-request base.

Workflow run 39276239077, job 118141273909: 403 passed, 1 failed. The failure is case::load_csv_integer_endpoint_memory.gqt:

timeout: worker exceeded wall-time budget 4.883008336s; original poll=Ok(None); contained=true
timeout: case exceeded wall-time budget 10s

That case is absent from this branch. It is on base 2eeae2049f379fc06fd097479a8a013ac32109dc (#1085), at the 65,536-key shape, with timeout_ms: 10000. The push of that base already failed the same case in run 39261877220, job 118091437212, with the same 10 second case timeout. #1072 CI is green because its base 0a62bb7b does not contain the case. Current main 9e2425d2 (#1087) shrinks the case to 8 keys. GQ Logic Tests on that commit succeeded (runs 39295616644 and 39294653114).

git merge-tree --write-tree of 9e2425d2 and bb2857ff exits 0 and writes tree 468a656e3e861d0fea0f1954237b2f5cd728affd. I did not compile that tree.

A logic edit in the diagnostic or budget commits will not clear this red check. Rebase, then treat a new green GQT (dst) as the merge gate.

Optional

1. Define partitions_ranked, and name the Lance counters this door omits.

In Lance 13.0.0 (tag v13.0.0, commit 0923349baa19fd9af776b7d4626dc758dc610c80), knn.rs adds index.total_partitions() to partitions_ranked once for each opened vector index. The value is the IVF list size, summed across deltas. It is a list-size total. partitions_searched is the number of probes that ran. nprobes stays on the probe attempt as maximum_nprobes.

from_summary reads all_counts for six names, and all_times for IndexTiming::ALL (nine names) plus prefilter_load_time, prefilter_input_time, and prefilter_build_time. collect_execution_metrics stores parts_loaded, index_comparisons, and indices_loaded on typed fields, so those three never enter all_counts. index_cache_hits, index_cache_misses, deltas_searched, and find_partitions_elapsed are also outside this projector. docs/user/queries/explain.md describes the curated set and says missing metrics stay absent. It does not define partitions_ranked. An operator who uses that number as a recall numerator will misread the scan.

OmniGraph builds only VectorIndexParams::ivf_flat (table_store.rs). The execution summary carries no IVF nlist, PQ, or HNSW build parameters, so this door cannot report them. One HNSW search path in Lance accepts the metrics argument and records nothing. Current OmniGraph builds do not use that path.

2. State the default buffer against Lance's own default, and correct the V2_2 sentence.

budgeted_file_writer_options passes the whole write_max_bytes value as data_cache_bytes. Lance divides that value by the top-level field count. When the option is absent, the file writer uses 8 MiB per column. At the 32 MiB default, a table with fewer than four top-level fields buffers more per column than that Lance default. A wider table buffers less. A value smaller than the field count becomes 0 per column, and the V2.2 encoder then flushes every nonempty array. The array that crosses the threshold is kept whole. Nested struct and list children each receive that same per-column share, so leaf fan-out multiplies the queues. stage_write_concurrency (default 8) gives every table writer the full target.

The setting remains the admission limit on each write account. On these two paths it is also a flush threshold. Process RSS stays outside this knob. max_rows_per_group has no effect on V2.2 files. max_bytes_per_file stays Lance's soft file-size default, checked after a group. Both budgeted WriteParams leave data_storage_version unset, so an existing dataset keeps its manifest version. docs/dev/writes.md says "The write still selects V2_2". Dataset create still pins V2_2. These two sites do not select the version again.

keep_original_array: Some(false) matches the file-writer path (encoding_options uses unwrap_or(false)). max_page_bytes: None matches V2.2: the primitive structural encoder does not read that field.

MergeInsertBuilder in Lance 13.0.0 has no writer-options setter. Its internal WriteParams shim is Default. The documented gap for upserts and updates matches the pinned crate. A later Lance release needs a public setter before this budget can cover merge-insert. A local Lance patch is outside this repository's rules.

3. Attach the storage-cost claim to the follow-up slice.

Against 91ecceef, the follow-up appends six golden rows. OverwriteWriter128KiB and OverwriteWriter32MiB each record l.get 13, l.list 1, and l.put 6. Equal request counts measure requests. They do not measure RSS or throughput. The page-count test in staged_tests.rs is the layout evidence: the hot column's page count at 128 KiB versus 32 MiB. Against main, the Lance 13 commits in this same branch also change existing l.get rows. "Every existing golden row is unchanged" is true for the two-commit slice. It is false for the diff against main.

Contract and liability

Query rows, errors, and probe decisions keep the previous contract. The new profile rows are additive. Replay ignores them. That split is required: the timings overlap, they include waits, their sum is not query latency, and they can change between runs.

write_max_bytes gains a second effect on strict inserts and overwrites. Logical admission stays as it was. New data files can change page layout. Page layout is derived performance state. The tests check layout and request counts, and they leave logical rows on the existing assertions.

The execution report is the stored diagnostic. Executed::profile derives the public rows. CLI, HTTP, OpenAPI, and MCP do not return this report. The documented door is Session::query_inspected. GQT keeps a second name list (ANN_COUNTS, ANN_TIMES) with unknown names rejected. The comment on that list says the copy is an independent contract check. diagnostics present only requires report.has_ann for the scan id. The Rust test issue_567_bounded_nearest_and_rrf_retry_after_partitioned_ivf_underfill checks partitions_searched against instrumentation, requires index_partition_load_time and index_search_time, and caps the maps at 6 counts and 12 times. A later diagnostic family should extend this report. A parallel map on the HTTP response would be a second source.

#1072 is the parent. The knobs map onto Lance 13.0.0: FileWriterOptions.data_cache_bytes, IndexTiming::ALL, and the three prefilter time names exist at that pin. The textual merge with current main keeps #1085's integer key-set files, because this branch does not edit them. I found no other open pull request that edits AnnAttemptMetrics or budgeted_file_writer_options.

This slice has no bug-fix regression to add. It does not repair a row-visible defect. I added no temporary .gqt case.

Validation

  • Head: bb2857ffdf394a34a6608cf6c9760dbdc0c15469. The pull request is open. Base: 2eeae2049f379fc06fd097479a8a013ac32109dc. Reviews: 0. Issue comments: 0. Review comments: 0.
  • Inspection used an isolated checkout of that commit, crates.io Lance 13.0.0, and Git tag v13.0.0 at 0923349baa19fd9af776b7d4626dc758dc610c80. Live lance.org pages were not the authority.
  • Exact-head CI: GQ Logic Tests run 39276239077 failed in GQT (dst) as quoted. GQT (ordinary) and GQT (served) passed. CI run 39276239339 passed, including workspace tests, clippy, rustfmt, S3, Azure, and the storage-upgrade job. DST run 39276239098 passed, including the pinned suite and the new golden rows.
  • Local limits: no Lance 13 rebuild and no GQT run in this review. The merge-tree with 9e2425d2 was clean and was not compiled.

@aaltshuler
aaltshuler force-pushed the codex/lance-ann-writer-budgets branch from bb2857f to b3a2bed Compare October 20, 2026 13:52
@aaltshuler
aaltshuler added this pull request to the merge queue Oct 20, 2026
Merged via the queue into ModernRelay:main with commit 4ba904d Oct 20, 2026
38 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