alamb commented on code in PR #7538:
URL: https://github.com/apache/arrow-datafusion/pull/7538#discussion_r1324475746
##########
datafusion/execution/src/disk_manager.rs:
##########
@@ -125,32 +126,61 @@ impl DiskManager {
// Create a temporary directory if needed
if local_dirs.is_empty() {
- let tempdir = temp_dir();
+ let tempdir =
tempfile::tempdir().map_err(DataFusionError::IoError)?;
debug!(
- "Use directory '{:?}' as DataFusion tempfile directory for {}",
- tempdir.to_string_lossy(),
+ "Created directory '{:?}' as DataFusion tempfile directory for
{}",
+ tempdir.path().to_string_lossy(),
request_description,
);
- local_dirs.push(tempdir);
+ local_dirs.push(Arc::new(tempdir));
}
let dir_index = thread_rng().gen_range(0..local_dirs.len());
- Builder::new()
- .tempfile_in(&local_dirs[dir_index])
- .map_err(DataFusionError::IoError)
+ Ok(RefCountedTempFile {
+ parent_temp_dir: local_dirs[dir_index].clone(),
+ tempfile: Builder::new()
+ .tempfile_in(local_dirs[dir_index].as_ref())
+ .map_err(DataFusionError::IoError)?,
+ })
}
}
-/// Ensure local dirs present
-fn create_local_dirs(local_dirs: &[PathBuf]) -> Result<()> {
- local_dirs.iter().try_for_each(|root| -> Result<()> {
- if !std::path::Path::new(root).exists() {
- std::fs::create_dir(root)?;
- }
- Ok(())
- })
+/// A named temporary file and the directory it lives in, which may be clean
up on drop
Review Comment:
```suggestion
/// A wrapper around a [`NamedTemporaryFile`] that also contains a reference
/// to the temporary directory it is in. The file is cleaned up on drop.
```
##########
datafusion/execution/src/disk_manager.rs:
##########
@@ -125,32 +126,61 @@ impl DiskManager {
// Create a temporary directory if needed
if local_dirs.is_empty() {
- let tempdir = temp_dir();
+ let tempdir =
tempfile::tempdir().map_err(DataFusionError::IoError)?;
debug!(
- "Use directory '{:?}' as DataFusion tempfile directory for {}",
- tempdir.to_string_lossy(),
+ "Created directory '{:?}' as DataFusion tempfile directory for
{}",
+ tempdir.path().to_string_lossy(),
request_description,
);
- local_dirs.push(tempdir);
+ local_dirs.push(Arc::new(tempdir));
}
let dir_index = thread_rng().gen_range(0..local_dirs.len());
- Builder::new()
- .tempfile_in(&local_dirs[dir_index])
- .map_err(DataFusionError::IoError)
+ Ok(RefCountedTempFile {
+ parent_temp_dir: local_dirs[dir_index].clone(),
+ tempfile: Builder::new()
+ .tempfile_in(local_dirs[dir_index].as_ref())
+ .map_err(DataFusionError::IoError)?,
+ })
}
}
-/// Ensure local dirs present
-fn create_local_dirs(local_dirs: &[PathBuf]) -> Result<()> {
- local_dirs.iter().try_for_each(|root| -> Result<()> {
- if !std::path::Path::new(root).exists() {
- std::fs::create_dir(root)?;
- }
- Ok(())
- })
+/// A named temporary file and the directory it lives in, which may be clean
up on drop
+#[derive(Debug)]
+pub struct RefCountedTempFile {
+ /// directory in which temporary files are created
Review Comment:
```suggestion
/// directory in which temporary files are created (Arc is held to ensure
/// it is not cleaned up prior to the NamedTempFile)
```
--
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]
