fix(ci): enlarge mem pool only around merge_insert index build - #8191
Conversation
The sort spill reservation was sized as pool/3 but hard-capped at 40 MiB. Under the default 150 MiB FairSpillPool that leaves only 40 MiB free for ExternalSorterMerge, while BTREE training on large string columns (e.g. 10M uuid keys in merge_insert datagen) peaks near 48 MiB and fails with ResourcesExhausted. Raise the ceiling to 64 MiB so the default pool keeps its intended pool/3 (= 50 MiB) reservation.
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
The reservation increase matches DataFusion’s merge-memory contract. The remaining risk is that the exact workload threshold and added spill I/O are not behaviorally pinned; an end-to-end regression that fails at 40 MiB and passes at 50 MiB would make this tuning durable.
| } | ||
|
|
||
| #[test] | ||
| fn test_sort_spill_reservation_bytes() { |
There was a problem hiding this comment.
This validates the sizing arithmetic, but not the behavioral contract that 40 MiB exhausts the target workload and 50 MiB succeeds. I ran a 10M-row UUID7-shaped, 32-byte-string BTREE build against the base 150 MiB / 40 MiB configuration and it passed, so the failure depends on a batch or storage shape this test does not capture. Please add an end-to-end case matching that shape, or capture the relevant merge-reservation metrics, so the threshold is pinned.
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
I'm not opposed but I'm not sure this will work any better. There is just no safe way to set these variables. It seems pretty much impossible to predict what sort_spill_reservation_bytes should be set to.
I'm not really won't to set this. Just wanna to fix the bench regress CI. I'm open to put a step back for better solutions. cc @wjones127 to join the discuss. |
I'm fine with just fixing this in CI. I'm sure you and Weston have looked more at this, so it seems like there isn't a great generate solution for sorting wide data with small memory pools. I think in production most folks will just want to increase the memory pool to be a significant fraction of system memory, at least in single-tenant use cases. |
BTREE training on merge_insert_narrow's 10M string keys can exhaust the default 150 MiB FairSpillPool during ExternalSorterMerge and abort bench_regress datagen. Raising the library-wide sort spill reservation is not a reliable fix for this class of failure. Enlarge LANCE_MEM_POOL_SIZE to 1 GiB only for the create_scalar_index loop in merge_insert datagen, then restore the previous env so measured benchmark paths keep the default pool.
Xuanwo
changed the title
3 participants
Why
Run Regression Benchmarksfails during Generate datasets when building BTREE indexes onmerge_insert_narrowstring keys:Example: https://github.com/lance-format/lance/actions/runs/30819924543/job/91706969640
Approach
Library-wide tuning of
sort_spill_reservation_bytesis not a reliable fix for this class of failure (there is no safe general value for that knob). Index build here is one-shot datagen setup, not a measured benchmark path.Temporarily set
LANCE_MEM_POOL_SIZE=1GiBonly around thecreate_scalar_indexloop in merge_insert datagen, then restore the previous env so measured paths keep the default pool.Notes