raghavyadav01 opened a new pull request, #26583:
URL: https://github.com/apache/datafusion/pull/26583
## Which issue does this PR close?
Closes #26446.
## Rationale for this change
`DiskManagerBuilder::build` in `DiskManagerMode::Directories` called
`create_local_dirs`, so a `datafusion-*` working directory was created in every
configured directory as soon as the `DiskManager` was built, and torn down
recursively when it was dropped — whether or not the query ever spilled.
`DiskManagerMode::OsTmpDirectory` already defers this to the first
`create_tmp_file` (#1695). This makes `Directories` behave the same way.
The issue measures the cost: 11 syscalls per configured directory per
`DiskManager` on Linux, and in DataFusion Comet — which builds a `RuntimeEnv`
per 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?
`DiskManager` now carries the configured paths in a `conf_dirs` field and
creates the working directories on the first `create_tmp_file`. `build()` no
longer touches the filesystem in any mode.
`conf_dirs` is 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.
- An unusable configured directory now fails the first spill rather than
`build()`.
## Are these changes tested?
Yes — `test_directories_mode_creates_dirs_lazily` builds a `DiskManager`
over a configured directory, asserts that directory is still empty and that
`temp_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 `main` with:
```
expected no directories before the first spill,
found [Ok(DirEntry(".../datafusion-wWF7BS"))]
```
Also run: the full `datafusion-execution` suite (122 + 20 passing), the
`datafusion-physical-plan` spill tests (159 passing), `cargo fmt --all --
--check`, and `cargo 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
