perf: increase default memory pool to 150MB with 40MB sort spill reservation#7675
Conversation
…rvation Raises the per-partition memory pool from 100MB to 150MB and sets the sort spill reservation to 50MB (up from the DataFusion default of 10MB) to give sort operations more headroom before spilling to disk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the fixed 50MB sort_spill_reservation_bytes constant with a dynamic value computed as pool_size / 3, so the reservation scales proportionally when the pool size is overridden via option or env var. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts default DataFusion execution memory settings in lance-datafusion to reduce false “resources exhausted” errors during sort spilling by increasing the available per-partition pool and reserving more headroom for spill/merge overhead.
Changes:
- Increase the default per-partition memory pool from 100MB to 150MB.
- Configure DataFusion’s
sort_spill_reservation_bytestomem_pool_size / 3(50MB with the new default).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let sort_spill_reservation_bytes = (options.mem_pool_size() / 3) as usize; | ||
| let mut session_config = SessionConfig::new() | ||
| .with_sort_spill_reservation_bytes(sort_spill_reservation_bytes); | ||
| let mut runtime_env_builder = RuntimeEnvBuilder::new(); |
hamersaw
left a comment
There was a problem hiding this comment.
This sounds reasonable.
|
I ended up tweaking things slightly. One set of tests was failing because 3 tests ran at once and tried to create an index. Each test grabbed 50MB of headroom from the same shared pool and then none of them could proceed. I dropped the limit down to 40, which is a somewhat arbitrary way to make this pass. In addition, I made 40MB the max. If we keep scaling the headroom with the pool size then we have an effective cap on the max number of concurrent operations (something less than pool size / head room) which I don't think we want. Hopefully the headroom needs do not grow linearly with the pool size or else I think we will just have to fix the upstream. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Raises the per-partition memory pool from 100MB to 150MB and sets the sort spill reservation to 40MB (up from the DataFusion default of 10MB) to give sort operations more headroom while spilling to disk (we should still spill at roughly the same rate).
The previous defaults were 100MB / 10MB. This usually worked but certain patterns would lead to false memory exhaustion errors:
The problem happens as follows:
Both the cursors and the accumulation require additional space. This is the
ExternalSorterMerge[0]mentioned above. It is given the overcounting described in step 1. In other words, once this starts, we have half the reservation inExternalSorter[0]and half the reservation inExternalSorterMerge[0]. This "additional space" should be about the same size as the input. This is why we count each batch twice.In practice, the
ExternalSorterMerge[0]reservation ends up being slightly higher (for various reasons). This is what thesort_spill_reservation_bytesis supposed to account for. Datafusion defaults this to 10MB. There is no guidance (and I can't get Claude to come up with any good guidance) as to what this value should be set to. However, 10MB seems like too little. This PR updates it to about 1/3 of the memory pool size. In theory it shouldn't grow proportionally to the memory pool but in practice it seems to. I also really don't want to expose it as yet another knob that users have to tune so I'm hoping 1/3 is slightly conservative but good enough.