Repository navigation
perf: create DiskManager Directories-mode working dirs lazily - #26583
Open
raghavyadav01 wants to merge 2 commits into
Open
raghavyadav01 wants to merge 2 commits into
raghavyadav01 wants to merge 2 commits into
Conversation
DiskManagerBuilder::build in DiskManagerMode::Directories called create_local_dirs, so a datafusion-* directory was created in every configured directory when the DiskManager was built, and removed recursively when it was dropped, whether or not anything spilled. An engine that builds a RuntimeEnv per query or per task paid for that every time. Carry the configured paths on the DiskManager instead and create the working directories on the first create_tmp_file, as DiskManagerMode::OsTmpDirectory already does. Two visible differences, both noted on the issue: temp_dir_paths() returns nothing until the first spill, and an unusable configured directory now fails the first spill rather than build(). Closes apache#26446
raghavyadav01
force-pushed
the
perf/lazy-disk-manager-dirs
branch
from
October 22, 2026 02:16
84b6b95 to
c8390c4
Compare
kumarUjjawal
approved these changes
Oct 22, 2026
kumarUjjawal
left a comment
Contributor
There was a problem hiding this comment.
Thank you @raghavyadav01
LGTM!
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.
Which issue does this PR close?
Closes #26446.
Rationale for this change
DiskManagerBuilder::buildinDiskManagerMode::Directoriescalledcreate_local_dirs, so adatafusion-*working directory was created in every configured directory as soon as theDiskManagerwas built, and torn down recursively when it was dropped — whether or not the query ever spilled.DiskManagerMode::OsTmpDirectoryalready defers this to the firstcreate_tmp_file(#1695). This makesDirectoriesbehave the same way.The issue measures the cost: 11 syscalls per configured directory per
DiskManageron Linux, and in DataFusion Comet — which builds aRuntimeEnvper native plan in a Spark task — a query of 501 short tasks over 8 local directories ran 12% faster in wall time with 9% less CPU once the directories were created lazily.What changes are included in this PR?
DiskManagernow carries the configured paths in aconf_dirsfield and creates the working directories on the firstcreate_tmp_file.build()no longer touches the filesystem in any mode.conf_dirsis empty for every other mode, so the existing "no directories yet" branch keeps using an OS temporary directory and that behaviour is unchanged.Two visible differences, both called out on the issue:
temp_dir_paths()returns nothing until the first spill. The one in-tree caller,RuntimeEnv::describe(runtime_env.rs:294), already treats an empty list as "not set", so its output is unchanged until a spill happens.build().Are these changes tested?
Yes —
test_directories_mode_creates_dirs_lazilybuilds aDiskManagerover a configured directory, asserts that directory is still empty and thattemp_dir_paths()is empty, then creates a temporary file and asserts the working directory appears underneath the configured path.I wrote it first and confirmed it fails on
mainwith:Also run: the full
datafusion-executionsuite (122 + 20 passing), thedatafusion-physical-planspill tests (159 passing),cargo fmt --all -- --check, andcargo clippy -p datafusion-execution --all-targets -- -D warnings, all clean.Are there any user-facing changes?
No API change. The two behavioural differences above are user-visible:
temp_dir_paths()is empty until the first spill, and a bad configured directory surfaces at first spill instead of at build time.