test(datafusion): guard sort merge pool headroom - #9408
lance-gatefixer[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The underlying partition/pool mismatch was already corrected by #9183; this adds a focused runtime-level regression guard. It exercises the actual bounded pool, detects the former 150 MiB failure on multi-core defaults, and adds no production behavior beyond naming the existing 40 MiB cap.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
Re-review after merging main: the pull request patch is unchanged. #9183 already corrected the partition/pool mismatch; this remains a focused runtime-level regression guard that detects the former 150 MiB failure without changing production behavior beyond naming the existing 40 MiB cap.
|
Blocked: The current-head rustdoc check fails on an inherited private intra-doc link from current main. Remote head e785b2c contains current base tip 83d0084; rustdoc job 105692983695 reports that public documentation for with_k links to private Self::try_new_batch, and the base-tip rustdoc job 105689817498 reports the identical diagnostic. I fetched both refs, verified the base ancestry, compared the implicated source between base and repair head, and confirmed the repair patch does not modify it. The smallest next action is to merge a correction for the upstream #9334 documentation link into main and then update/rerun this PR; alternatively, explicitly authorize carrying that unrelated one-line documentation correction on this repair branch. |
Root cause
The failing workflow checked merge commit
34859c20016e015ece2f3cd6ad2295cac99fc326, whose base (fea83fea125ea1cb544fdbb8effb94084e71a7ee) predated the partition-aware pool sizing fix inaac7844ee4c9b2ffd7d28428f0f292fe9cceef55. The session could therefore run one DataFusion partition per available core while retaining a single 150 MiB pool. ConcurrentExternalSorterMergeconsumers each need up to 40 MiB of non-spillable headroom, so the fourth merge exhausted that pool with only 30 MiB remaining.Fix
The runtime now sizes the default pool from the effective partition count. This change makes the 40 MiB cap a named constant and adds execution-level regression coverage against the actual bounded session pool. The test holds three merge reservations per effective partition, preserving 30 MiB per partition for input batches and other operators; the historical single-partition pool fails this scenario on a multi-core runner.
Validation
cargo test -p lance-datafusion(182 unit tests and 5 doctests passed; 3 pre-existing ignored doctests)cargo test -p lance --features slow_tests --test integration_tests query::primitives::test_query_integer(8 passed, includingcase_7_uint32)cargo fmt --all -- --checkcargo clippy --all --tests --benches -- -D warningsFixes #9402