Shockang commented on a change in pull request #33101:
URL: https://github.com/apache/spark/pull/33101#discussion_r663302600
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -285,9 +285,10 @@ private[spark] object Utils extends Logging {
*/
def createDirectory(dir: File): Boolean = {
try {
- // This sporadically fails - not sure why ... !dir.exists() &&
!dir.mkdirs()
- // So attempting to create and then check if directory was created or
not.
- dir.mkdirs()
+ // SPARK-35907
+ // This could throw more meaningful exception information if directory
creation failed.
+ // To be on the safe side, try to create and then check if directory was
created or not.
+ Files.createDirectories(dir.toPath)
if ( !dir.exists() || !dir.isDirectory) {
logError(s"Failed to create directory " + dir)
}
Review comment:
> @Shockang Could you leave a comment above the check to give the
historical context, e.g.,
>
> "The check was required by File#mkdirs() because it could sporadically
fail silently. After switching to Files.createDirectories(), ideally, there
should no longer be silent fails. But the check is kept for the safety concern.
We can remove the check when we're sure that Files.createDirectories() would
never fail silently."
This comment looks perfect. Thank you very much for your comments.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
