Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3670#discussion_r24038361
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -513,13 +516,44 @@ private[spark] object Utils extends Logging {
Files.move(sourceFile, destFile)
--- End diff --
We talked about this offline; it seems that, in Spark's case, both source
and destination are on the same filesystem, so this should work.
Guava does something akin to `if (!from.renameTo(to)) copy(from, to)`
internally; that works on Unix for directories if both source and destination
are on the same filesystem. On Windows, though, it would fail: `renameTo` fails
on Windows if the destination already exists, and the Guava code would revert
back to `copy()` which only works on files. (That also means `move` is
unnecessarily expensive on Windows then the destination exists, but that's a
Guava issue.)
So if we care about Windows here we should probably do a
`deleteRecursively` before trying the rename. It ceases being an "atomic"
operation, but Java doesn't expose the Window's version of
[`rename`](https://msdn.microsoft.com/en-us/library/zw5t957f.aspx), which
follows the Unix semantics.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
