Skip to content

fix(ci): run full-fragment merge_insert benchmarks on v2 only - #8260

Draft
wjones127 wants to merge 1 commit into
lance-format:mainfrom
wjones127:fix/merge-insert-bench-mem-pool
Draft

fix(ci): run full-fragment merge_insert benchmarks on v2 only#8260
wjones127 wants to merge 1 commit into
lance-format:mainfrom
wjones127:fix/merge-insert-bench-mem-pool

Conversation

@wjones127

Copy link
Copy Markdown
Contributor

The Run Regression Benchmarks job has failed on every push to main since the merge_insert benchmark suite landed in #8052. Seven cases fail with Resources exhausted, all of them the 100pct-v1_indexed shapes against the wide target.

merge_insert has two execution paths, and the wide benchmarks are parametrized on both. The legacy indexed path builds its hash join on the source side, so at the 100pct row fraction — where the source covers all 1M rows of the wide target — it asks for the entire source at once. That is 137 MB for a single scalar column and about 1.3 GB once the 256-dim vector column is projected, against a 150 MB pool, so the operation cannot run at all.

This is a known limitation rather than a new one: #8052 described it and already kept test_upsert_source_equals_target on the v2 path for exactly this reason. It was missed for the wide benchmarks because they were verified locally against a scaled-down 20K-row target, where a full-fraction source still fits in the pool.

This PR applies the same exclusion to the wide sweeps, so full-fragment shapes report a v2 number only. The 10pct fraction still gives a v1-vs-v2 write-amplification comparison at fragment scale, which is what the write-path group is there to measure.

Every retained case keeps the test id it has today, so the benchmark history in Bencher stays continuous rather than restarting under new names.

Not included

Teaching the legacy indexed path to handle a source the size of its target. Its source-side hash join, and the unbounded replay buffer that feeds it, would both need to change; that is a performance change to a legacy path rather than a CI fix.

Testing

.github/workflows/ci-benchmarks.yml runs only on workflow_dispatch and on push to main, so PR CI does not exercise any of this. Verified locally instead that the collected case set drops exactly the seven failing 100pct-v1_indexed cases and that every remaining case keeps a byte-identical test id.

The wide merge_insert benchmarks were parametrized on every combination of
row fraction and execution path, including a 100pct source on the legacy
indexed path. That path builds its hash join on the source side, so a source
covering all 1M rows of the wide target requests between 137 MB and 1.3 GB
against a 150 MB memory pool and fails with "Resources exhausted". Seven
cases failed on every main run since the suite landed.

Drop the 100pct x v1_indexed combinations, the same exclusion that already
keeps test_upsert_source_equals_target on the v2 path only. Every retained
case keeps its existing test id so the Bencher history is unbroken.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added bug Something isn't working A-python Python bindings labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant