Skip to content

feat: expose structured merge diagnostics and spill accounting - #10752

Merged
Xuanwo merged 4 commits into
lance-format:mainfrom
kamronis:feat/structured-metrics-and-spill-telemetry
Nov 14, 2026
Merged

Xuanwo merged 4 commits into
lance-format:mainfrom
kamronis:feat/structured-metrics-and-spill-telemetry

Conversation

@kamronis

@kamronis kamronis commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

feat: expose structured merge diagnostics and spill accounting

Merge-insert analysis currently requires parsing plan text and omits separately executed indexed joins and fragment-update sorts. Add structured Rust, Python, and Java reports for current plan-based and scalar-index merge execution. Analysis uses the existing writer and uncommitted transaction machinery, preserves source statistics and first-seen deduplication, and cleans up temporary data, deletion, and managed-blob files without advancing the dataset version. Execution failures return partial reports with complete change statistics absent.

Reports expose actual join/write strategies, logical changes, attributed scan visits, selected index segments, coarse mapped/joined row counts, overlapping join/inclusive write timings, and operation DataFusion pool peaks. Nested plans have unique operation-local identities and retain operator paths, partitions, labels, and metric kinds. Operator spills aggregate root and nested producers once; operation-owned replay traffic remains separate. Current merge routes allocate no scratch-store files, so scratch measurements remain unavailable rather than reporting unrelated session traffic. Java receives an opaque runtime protobuf and exposes typed accessors; no diagnostic metadata is persisted.

Terminal diagnostics finalize once on exhaustion, first stream error, or early drop, while the existing scan callback keeps its exhaustion-only contract. Scratch scopes retain independent traffic/lifetime accounting while sharing quota. Scratch gauges publish current process-wide totals, including spills created before recorder installation; registering metric descriptions publishes the existing usage immediately. Recorder regression tests exercise actual scratch traffic through the Python and Java OpenTelemetry bridges, including duration samples and gauges returning to zero. Operation and subplan identifiers correlate traces and reports without becoming metric labels.

Related to #9863. Pool reservations are not RSS or total Arrow/allocator memory. Producer times overlap; scan visits are not distinct target cardinalities. Failed/cancelled snapshots can precede outstanding I/O reclamation, and cancelling analysis can interrupt best-effort cleanup. Existing text APIs retain their signatures and eligibility. Legacy writers, session-cache diagnostics, process-memory sampling, ANN/FTS-wide candidate flow, and broad stage/progress instrumentation remain outside this PR.

Indexed null-key behavior remains the existing writer's behavior: a null source key is discarded. Regression coverage compares structured analysis with ordinary execution; this diagnostic change does not alter that behavior.

Maintenance update (2026-11-14, head 88ea060e0dac40fc879af7dfe9b9195961ecd7aa): merged current main and fixed two diagnostics findings on the indexed partial-schema path. An insert-only analysis no longer leaves its managed Blob payloads behind, and its report now reports rewrite_rows while the persisted transaction mode and ordinary execution are unchanged. Added Rust, Python, and Java regressions for insert-only, mixed, and no-op reports.

Validation

  • Full workspace cargo check --profile ci --locked --workspace --tests --benches and cargo clippy --profile ci --locked --all --tests --benches -- -D warnings passed.
  • Full workspace cargo test --profile ci --locked --workspace: 22,875 passed, 0 failed, 51 ignored.
  • Targeted merge-analysis runs covered indexed partial Blob cleanup for packed and dedicated payloads, insert-only/mixed/no-op strategy reports, tracing subprocess assertions, shared-slice replay, scalar address-mask semantics, and component gauges.
  • Python make install/make build, 24 structured-analysis, source-routing, and OpenTelemetry tests, and uv run make lint passed.
  • Java spotless:check, JNI Clippy, 31 merge-insert tests, and the full 691-test suite (26 integration tests skipped) passed.
  • Protobuf classifier/comment and format-vote CI script tests passed (37 tests).

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added A-python Python bindings A-java Java bindings + JNI A-encoding Encoding, IO, file reader/writer enhancement New feature or request labels Nov 5, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. and removed K-approved Latest Gatekeeper recommendation permits acceptance. labels Nov 5, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 5, 2026
@kamronis
kamronis force-pushed the feat/structured-metrics-and-spill-telemetry branch from 976537c to 1c969c5 Compare November 13, 2026 04:41
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 13, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 13, 2026
@kamronis
kamronis force-pushed the feat/structured-metrics-and-spill-telemetry branch from 1c969c5 to 2847ba7 Compare November 14, 2026 06:38
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 14, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 14, 2026
…ly writes

Analysis of an indexed partial-schema insert now records managed Blob payloads in the existing analysis inventory, so cleanup removes them. The report also names an insert-only execution as RewriteRows while leaving the persisted transaction mode and ordinary execution unchanged.
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 14, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Gate recommendation: approve.

Using the existing merge execution paths is a sound way to expose the costs described in #9863. I found no merge blocker in this revision.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Nov 14, 2026
@Xuanwo
Xuanwo merged commit cbed18e into lance-format:main Nov 14, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer A-java Java bindings + JNI A-python Python bindings enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants