feat: cap spill merge fan-in by Kevin-Li-2025 · Pull Request #23066 · apache/datafusion

Kevin-Li-2025 · 2026-06-21T11:17:03Z

Which issue does this PR close?

Closes Add a configurable cap on spill-file merge fan-in (max open files during external merge) #22848.

Rationale for this change

External sort merge phases currently select spill files based only on memory reservation. With many small spills, a single phase can open enough files to exceed the process file-descriptor limit.

What changes are included in this PR?

Add datafusion.runtime.max_spill_merge_fan_in (0 preserves the current unlimited behavior).
Clamp non-zero values to at least 2 during merge selection so each pass makes progress.
Support builder configuration and dynamic SQL SET / RESET / SHOW.
Add unit, runtime SQL, SQLLogicTest, information schema, and generated documentation coverage.

Are there any user-facing changes?

Users can cap the number of spill files opened in one external merge pass. The default remains unchanged.

How was this change tested?

cargo test -p datafusion-execution test_max_spill_merge_fan_in_builder_and_dynamic_update --lib
cargo test -p datafusion-physical-plan spill_merge_fan_in --lib
cargo test -p datafusion --test core_integration test_max_spill_merge_fan_in_runtime_config
cargo test -p datafusion-sqllogictest --test sqllogictests -- set_variable.slt
cargo check -p datafusion
cargo clippy -p datafusion-execution -p datafusion-physical-plan -p datafusion --lib -- -D warnings
cargo fmt --all -- --check
dev/update_config_docs.sh

kosiew

@Kevin-Li-2025
Looks good to me. I left one small suggestion that could make the test coverage a bit more direct.

kosiew · 2026-06-25T07:18:19Z

        let mut total_needed: usize = 0;

        for spill in &self.sorted_spill_files {
+            if number_of_spills_to_read_for_current_phase >= max_spill_files {


Nice coverage on the helper cases for 0/1/2/N. One extra test could make this even stronger: build multiple SortedSpillFiles with max_spill_merge_fan_in = 2 and assert that a merge phase selects only two spill inputs. That would guard the FD-limit regression more directly than the current config and helper coverage.

Addressed in e564acd: added spill_merge_phase_respects_configured_fan_in, which builds four SortedSpillFiles with max_spill_merge_fan_in = 2 and asserts one merge phase selects exactly two spill inputs while leaving two for the next phase.

Kevin-Li-2025 · 2026-06-25T15:39:24Z

Addressed the test coverage suggestion in 8315dce. I added a focused unit test that builds four SortedSpillFile entries with max_spill_merge_fan_in = 2 and asserts that one merge phase selects/drains only two spill inputs, leaving the remaining two for a later phase.\n\nLocal validation: git diff --check passed. I could not run cargo fmt/test in this sandbox because rustup tries to write under ~/.rustup/update-hashes, which is not writable here, and a temporary minimal toolchain download stalled before completion.

Kevin-Li-2025 · 2026-06-25T15:42:24Z

Rebased onto current apache/datafusion main and resolved the conflict in disk_manager.rs by keeping the current builder-only DiskManager construction path; the spill fan-in setting is still wired through DiskManagerBuilder. New head: e564acd. Validation: git diff --check origin/main...HEAD.

kosiew · 2026-06-29T05:52:10Z

@Kevin-Li-2025
Can you resolve the cargo fmt errors?

kosiew · 2026-06-29T07:44:50Z

@Kevin-Li-2025
Can you resolve the CI error?

Kevin-Li-2025 · 2026-06-29T13:29:45Z

Pushed f36a910 to address the remaining sqlite sqllogictest failure.

The failed CI run showed the dynamic filter threshold as column1@0 > 3, while the same focused test can produce column1@0 > 4 locally. That matches the existing note in the test: the partial aggregate dynamic-filter update races with scan progress under parallel execution. I changed this assertion to keep checking the stable invariant (DynamicFilter is present, dynamic row-group pruning is eligible, and metrics are suppressed) while ignoring the volatile threshold value and file-group text.

Local validation:

cargo test -p datafusion-sqllogictest --test sqllogictests -- push_down_filter_regression --include-sqlite
cargo fmt --all -- --check

alamb

Thank you @Kevin-Li-2025 and @kosiew -- I had some code structure questions, but the idea looks good to me

alamb · 2026-06-30T18:41:40Z


        let mut state = self.state.write();

+        if key == "max_spill_merge_fan_in" {


Why doesn't this follow the same pattern as the other variables (aka in the match statement below)?

alamb · 2026-06-30T18:41:56Z


        let mut state = self.state.write();

+        if key == "max_spill_merge_fan_in" {


ditto -- I think this should follow the same pattern as the other variabales

alamb · 2026-06-30T18:42:26Z

 use datafusion_common::human_readable_size;

 pub const DEFAULT_MAX_TEMP_DIRECTORY_SIZE: u64 = 100 * 1024 * 1024 * 1024; // 100GB
+pub const DEFAULT_MAX_SPILL_MERGE_FAN_IN: usize = 0;


I think limiting it to 100 files or something is probably reasonable. We can consider doing this as a follow on PR

alamb · 2026-06-30T18:43:49Z

-) -> Vec<ConfigEntry> {
+}
+
+fn create_runtime_config_entries(values: RuntimeConfigValues) -> Vec<ConfigEntry> {


How about making this a method on RuntimeConfigValues -- like

impl RuntimeConfigValues { fn into_config_entries(values: RuntimeConfigValues) -> Vec<ConfigEntry> { .. } .. }

Signed-off-by: Kevin-Li-2025 <2242139@qq.com>

alamb

Looks good to me -- thank you @kosiew and @Kevin-Li-2025

alamb · 2026-07-03T14:18:49Z

-/// entries to avoid duplication between `RuntimeEnv::config_entries()` and
-/// `RuntimeEnvBuilder::entries()`.
-fn create_runtime_config_entries(
+struct RuntimeConfigValues {


alamb · 2026-07-03T14:19:42Z

-            None,
-            Some("20M".to_owned()),
-        )
+        RuntimeConfigValues {


this is a nice refactoring into a struct rather than a function

github-actions Bot added documentation Improvements or additions to documentation core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) execution Related to the execution crate physical-plan Changes to the physical-plan crate labels Jun 21, 2026

kosiew approved these changes Jun 25, 2026

View reviewed changes

Kevin-Li-2025 force-pushed the feat/spill-merge-fan-in branch from 8315dce to e564acd Compare June 25, 2026 15:42

github-actions Bot added the auto detected api change Auto detected API change label Jun 30, 2026

alamb reviewed Jun 30, 2026

View reviewed changes

Kevin-Li-2025 added 5 commits July 1, 2026 15:12

feat: cap spill merge fan-in

8caeedc

Add spill merge fan-in phase test

ee82bbc

Signed-off-by: Kevin-Li-2025 <2242139@qq.com>

test: stabilize dynamic filter explain expectation

3779fd4

refactor: align runtime spill config handling

7fae109

Address spill fan-in review comments

33dc1b2

Signed-off-by: Kevin-Li-2025 <2242139@qq.com>

Kevin-Li-2025 force-pushed the feat/spill-merge-fan-in branch from a32c55b to 33dc1b2 Compare July 1, 2026 07:36

github-actions Bot removed the auto detected api change Auto detected API change label Jul 2, 2026

kosiew requested a review from alamb July 3, 2026 05:57

alamb approved these changes Jul 3, 2026

View reviewed changes

Merge branch 'main' into feat/spill-merge-fan-in

84c068d

alamb enabled auto-merge July 3, 2026 14:20

alamb added this pull request to the merge queue Jul 3, 2026

Merged via the queue into apache:main with commit b7e7b51 Jul 3, 2026
39 checks passed


		let mut state = self.state.write();

		if key == "max_spill_merge_fan_in" {

Uh oh!

Conversation

Kevin-Li-2025 commented Jun 21, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

How was this change tested?

Uh oh!

kosiew left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kevin-Li-2025 commented Jun 25, 2026

Uh oh!

Kevin-Li-2025 commented Jun 25, 2026

Uh oh!

kosiew commented Jun 29, 2026

Uh oh!

kosiew commented Jun 29, 2026

Uh oh!

Kevin-Li-2025 commented Jun 29, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kosiew left a comment •

edited

Loading