andygrove commented on code in PR #2797:
URL:
https://github.com/apache/datafusion-ballista/pull/2797#discussion_r4507916547
##########
ballista/executor/src/config.rs:
##########
@@ -226,12 +226,13 @@ pub struct Config {
)]
pub memory_pool_fraction: f64,
/// Maximum disk space spill files may use, per session runtime (shared by
- /// that session's tasks). When unset, DataFusion's default (100 GB)
- /// applies. Accepts the same values as `--memory-pool-size`.
+ /// that session's tasks). When unset, the space free on the work dir's
+ /// filesystem at startup, or DataFusion's default (100 GB) if that is
+ /// smaller. Accepts the same values as `--memory-pool-size`.
Review Comment:
Good call, reworded. One small correction to the suggested text though: it's
the *larger* of the two, not the smaller. The default never goes below
DataFusion's 100 GB, so with 1 TB free the limit is about 1 TB.
##########
ballista/executor/src/executor_process.rs:
##########
@@ -198,6 +199,52 @@ fn read_host_total_memory() -> Option<u64> {
(total > 0).then_some(total)
}
+/// DataFusion caps spill files at 100 GB per runtime unless told otherwise.
+/// An executor's work dir is usually a volume of its own, so when no limit is
+/// configured, let spills use the space free on it at startup (never less
+/// than DataFusion's default). Large joins and sorts at TPC-H SF1000 spill
+/// more than 100 GB per executor and would otherwise fail with "The used disk
+/// space during the spilling process has exceeded the allowable limit".
+fn default_spill_limit(work_dir: &str) -> Option<u64> {
+ let Some(free) = free_disk_space(work_dir) else {
+ warn!(
+ "Could not read the free space of {work_dir}; spill files stay
limited to DataFusion's default"
+ );
+ return None;
+ };
+ spill_limit_for_free_space(free)
+}
+
+fn spill_limit_for_free_space(free: u64) -> Option<u64> {
+ (free > DEFAULT_MAX_TEMP_DIRECTORY_SIZE).then_some(free)
+}
+
+/// Bytes available to unprivileged users on the filesystem holding `path`, or
+/// on the closest existing parent when `path` itself does not exist yet.
+fn free_disk_space(path: &str) -> Option<u64> {
Review Comment:
Switched to `sysinfo::Disks`, thanks! One wrinkle: on Linux sysinfo leaves
tmpfs and network mounts out of its list by default. So I match the work dir to
its disk by device id rather than path prefix. Otherwise a work dir on tmpfs
would get the free space of `/`. If no listed disk matches, we fall back to
DataFusion's default as before.
--
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]
