test(python): add merge_insert benchmarks to ci_benchmarks - #8052
Open
wjones127 wants to merge 1 commit into
Open
test(python): add merge_insert benchmarks to ci_benchmarks#8052wjones127 wants to merge 1 commit into
wjones127 wants to merge 1 commit into
Conversation
The ci_benchmarks suite had no merge_insert coverage, so there was no way to see how the legacy indexed path compares to the DataFusion path, or to catch a regression in either. Adds 111 parametrized benchmarks covering the source/target ratio sweep, key distribution, cold vs warm index cache, partial-column write amplification, clause shapes, target layouts, and streaming sources. Every benchmark that can run on both paths is parametrized on `use_index`, which is the only knob that selects between them today. Also adds a `setup=` hook to the `io_mem_benchmark` fixture, which previously had no way to reset state between runs -- required for any benchmark that mutates its dataset. 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.
merge_inserthas two execution paths — a legacy indexed-scan path and a newer DataFusion path — and theci_benchmarkssuite measured neither. There was no way to see how they compare, and no regression signal for either. The one existing merge_insert benchmark lives inpython/benchmarks/(not run in CI) and covers a single 1K-row update.This PR adds a
merge_insertbenchmark module toci_benchmarks, plus the datasets it needs. Every benchmark that can run on both paths is parametrized onuse_index, which is the only knob that selects between them today:use_index(True)on an indexed key takes the legacy path,use_index(False)disables the index gate and takes the DataFusion path. That gives a side-by-side number for each shape rather than a single blended one.Coverage, in groups:
write_bytesis the metric of interest.Timing benchmarks report through
pytest-benchmark; the cost-model and write-path groups also run under theio_memory_benchmarkfixture so IO counts, write bytes, and peak memory are tracked.Targets are mutated by definition, so each dataset carries a
merge_insert_basetag marking a pristine version. Benchmarks restore to that tag before every measured round, in an untimed setup hook, and the module teardown restores and cleans up the versions the run produced. Restoring from a tag rather than from "whatever the latest version was" means a crashed run cannot silently leave a drifted dataset behind for the next one.That required one change to shared infrastructure: the
io_memory_benchmarkfixture had no way to reset state between its runs, and it always made an unmeasured warmup call. It now takes an optionalsetupcallable, invoked before the warmup and before the measured run, and before IO stats are reset so the reset's own writes are never counted. Existing callers are unaffected.Example
Two results from a local run, which show the kind of signal this is meant to surface. Both are legacy path vs DataFusion path on the same data.
Single-row upsert into a 1.1M-row target — the legacy index probe wins by 6.6x:
Updating one scalar column across every row of a wide target — the legacy path patches the column while the DataFusion path rewrites whole rows, including the vector column:
Not included
test_upsert_source_equals_targetruns on the DataFusion path only. The legacy path cannot execute that shape at all — its source-side hash join requests more than the whole memory pool and the operation fails with "Resources exhausted" — so there is no baseline to compare against.Benchmarks around repeated steady-state upsert loops and concurrent writers are deliberately out of scope; they pull compaction policy and conflict resolution in with them.
Testing
.github/workflows/ci-benchmarks.ymlonly runs onworkflow_dispatchand on push tomain, so PR CI will not exercise any of this. Verified locally instead: ran all 111 cases against scaled-down datasets (1.1M-row narrow target, 20K-row wide target), covering dataset generation, the restore-before-every-round setup, the row-count assertions, and the teardown cleanup. All pass.Also confirmed that
dataset.io_stats_incremental()attributes merge_insert's writes when read through the handle captured before the commit, so theio_memory_benchmarkvariants report realwrite_bytesrather than zero.