fix(ci): run full-fragment merge_insert benchmarks on v2 only - #8260
Draft
wjones127 wants to merge 1 commit into
Draft
fix(ci): run full-fragment merge_insert benchmarks on v2 only#8260wjones127 wants to merge 1 commit into
wjones127 wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
Run Regression Benchmarksjob has failed on every push tomainsince the merge_insert benchmark suite landed in #8052. Seven cases fail withResources exhausted, all of them the100pct-v1_indexedshapes against the wide target.merge_inserthas 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 the100pctrow 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_targeton 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
10pctfraction 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.ymlruns only onworkflow_dispatchand on push tomain, so PR CI does not exercise any of this. Verified locally instead that the collected case set drops exactly the seven failing100pct-v1_indexedcases and that every remaining case keeps a byte-identical test id.