[SPARK-32172][CORE]Use createDirectory instead of mkdir #28997
Conversation
|
ok to test |
| if (!newDir.exists() && !newDir.mkdir()) { | ||
| throw new IOException(s"Failed to create local dir in $newDir.") | ||
| } | ||
| Files.createDirectory(newDir.toPath) |
There was a problem hiding this comment.
This code looks like a regression. Previously, if the directory exists, we don't throw IOException.
Now, this PR will throw FileAlreadyExistsException if the directory exists.
There was a problem hiding this comment.
Yeah, either exists() -> Files.createDirectory or just Files.createDirectories would do the trick. The side-effect of latter is that it would also create non-existence parent directories.
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Thank you for making a PR, @sidedoorleftroad . However, the code seems to have a regression. Did you test your code in your Spark jobs?
|
Test build #124981 has finished for PR 28997 at commit
|
|
The code has been modified. |
|
Test build #125048 has finished for PR 28997 at commit
|
|
@dongjoon-hyun Do you have more items to suggest? The change looks to be simple and straightforward, but want to double-check. |
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you, @sidedoorleftroad , @HeartSaVioR , and @srowen .
Merged to master for Apache Spark 3.1.0.
|
Hi, @sidedoorleftroad . Are you |
|
@dongjoon-hyun Yes, I am. |
|
Good. SPARK-32172 is assigned to you, @sidedoorleftroad . Thank you for your contribution. |
5 participants
What changes were proposed in this pull request?
Use Files.createDirectory() to create local directory instead of File.mkdir() in DiskBlockManager.
Many times, we will see such error log information like "Failed to create local dir in xxxxxx". But there is no clear information indicating why the directory creation failed.
When Files.createDirectory() fails to create a local directory, it can give specific error information for subsequent troubleshooting(also throws IOException).
Why are the changes needed?
Throw clear error message when creating directory fails.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
DiskBlockManagerSuite