[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2986: [GOBBLIN-1147]Use one dfsClient in FsDataWriter to to rename and exists check to av…

[GitBox <gi...@apache.org>]

ZihanLi58 commented on a change in pull request #2986:
URL: https://github.com/apache/incubator-gobblin/pull/2986#discussion_r425567371



##########
File path: gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java
##########
@@ -266,19 +268,25 @@ public static void renamePath(FileSystem fs, Path oldName, Path newName) throws
    * {@link FileSystem#rename(Path, Path)} returns False.
    */
   public static void renamePath(FileSystem fs, Path oldName, Path newName, boolean overwrite) throws IOException {
-    if (!fs.exists(oldName)) {
-      throw new FileNotFoundException(String.format("Failed to rename %s to %s: src not found", oldName, newName));
-    }
-    if (fs.exists(newName)) {
-      if (overwrite) {
-        HadoopUtils.moveToTrash(fs, newName);
-      } else {
-        throw new FileAlreadyExistsException(
-            String.format("Failed to rename %s to %s: dst already exists", oldName, newName));
+    //In default implementation of rename with rewrite option in FileSystem, if the parent dir of dst does not exist, it will throw exception,
+    //Which will fail some of our job unintentionally. So we only call that method when fs is an instance of DistributedFileSystem to avoid inconsistency problem

Review comment:
       No, here I mean if we direct call FileSystem.rename, if it's not a DistributedFileSystem, it will try to use the default implementation in FileSystem.class, which will throw exception when parent of dst doe not exist.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org