You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gobblin.apache.org by hu...@apache.org on 2020/08/06 22:21:39 UTC

[incubator-gobblin] branch master updated: [GOBBLIN-1227][GOBBLIN-1277] Treat AccessDeniedException in RenameRecursively as an existence indicator

This is an automated email from the ASF dual-hosted git repository.

hutran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new a9bd984  [GOBBLIN-1227][GOBBLIN-1277] Treat AccessDeniedException in RenameRecursively as an existence indicator
a9bd984 is described below

commit a9bd984748c6df1b55679910945d326b127ecd78
Author: Hung Tran <hu...@linkedin.com>
AuthorDate: Thu Aug 6 15:21:30 2020 -0700

    [GOBBLIN-1227][GOBBLIN-1277] Treat AccessDeniedException in RenameRecursively as an existence indicator
    
    Closes #3073 from htran1/rename_access_denied
---
 .../java/org/apache/gobblin/util/HadoopUtils.java  | 19 +++++++++++---
 .../org/apache/gobblin/util/HadoopUtilsTest.java   | 30 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java b/gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java
index 355b329..f077c04 100644
--- a/gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java
+++ b/gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java
@@ -27,6 +27,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
+import java.nio.file.AccessDeniedException;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map.Entry;
@@ -47,7 +48,6 @@ import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.LocalFileSystem;
 import org.apache.hadoop.fs.Options;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.PathFilter;
 import org.apache.hadoop.fs.RawLocalFileSystem;
 import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.fs.permission.FsPermission;
@@ -598,9 +598,20 @@ public class HadoopUtils {
       try {
 
         // Attempt to move safely if directory, unsafely if file (for performance, files are much less likely to collide on target)
-        boolean moveSucessful =
-            this.from.isDirectory() ? safeRenameIfNotExists(this.fileSystem, this.from.getPath(), this.to)
-                : unsafeRenameIfNotExists(this.fileSystem, this.from.getPath(), this.to);
+        boolean moveSucessful;
+
+        try {
+          moveSucessful = this.from.isDirectory() ? safeRenameIfNotExists(this.fileSystem, this.from.getPath(), this.to) : unsafeRenameIfNotExists(this.fileSystem, this.from.getPath(), this.to);
+        } catch (AccessDeniedException e) {
+          // If an AccessDeniedException occurs for a directory then assume that it exists and continue the
+          // recursive renaming. If the error occurs for a file then re-raise the exception since the existence check
+          // is required to determine whether to copy the file.
+          if (this.from.isDirectory()) {
+            moveSucessful = false;
+          } else {
+            throw e;
+          }
+        }
 
         if (!moveSucessful) {
           if (this.from.isDirectory()) {
diff --git a/gobblin-utility/src/test/java/org/apache/gobblin/util/HadoopUtilsTest.java b/gobblin-utility/src/test/java/org/apache/gobblin/util/HadoopUtilsTest.java
index 4dc3ba6..a62193b 100644
--- a/gobblin-utility/src/test/java/org/apache/gobblin/util/HadoopUtilsTest.java
+++ b/gobblin-utility/src/test/java/org/apache/gobblin/util/HadoopUtilsTest.java
@@ -19,6 +19,7 @@ package org.apache.gobblin.util;
 
 import java.io.IOException;
 import java.net.URI;
+import java.nio.file.AccessDeniedException;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -96,6 +97,35 @@ public class HadoopUtilsTest {
 
   }
 
+  @Test
+  public void testRenameRecursivelyWithAccessDeniedOnExistenceCheck() throws Exception {
+    final Path hadoopUtilsTestDir = new Path(Files.createTempDir().getAbsolutePath(), "HadoopUtilsTestDir");
+    FileSystem fs = Mockito.spy(FileSystem.getLocal(new Configuration()));
+    Path targetDir = new Path(hadoopUtilsTestDir, "testRename");
+
+    // For testing that the rename works when the target
+    Mockito.doThrow(new AccessDeniedException("Test")).when(fs).exists(targetDir);
+
+    try {
+      fs.mkdirs(hadoopUtilsTestDir);
+
+      fs.mkdirs(new Path(hadoopUtilsTestDir, "testRename/a/b/c"));
+
+      fs.mkdirs(new Path(hadoopUtilsTestDir, "testRenameStaging/a/b/c"));
+      fs.mkdirs(new Path(hadoopUtilsTestDir, "testRenameStaging/a/b/c/e"));
+      fs.create(new Path(hadoopUtilsTestDir, "testRenameStaging/a/b/c/t1.txt"));
+      fs.create(new Path(hadoopUtilsTestDir, "testRenameStaging/a/b/c/e/t2.txt"));
+
+      HadoopUtils.renameRecursively(fs, new Path(hadoopUtilsTestDir, "testRenameStaging"), targetDir);
+
+      Assert.assertTrue(fs.exists(new Path(hadoopUtilsTestDir, "testRename/a/b/c/t1.txt")));
+      Assert.assertTrue(fs.exists(new Path(hadoopUtilsTestDir, "testRename/a/b/c/e/t2.txt")));
+    } finally {
+      fs.delete(hadoopUtilsTestDir, true);
+    }
+
+  }
+
   @Test(groups = { "performance" })
   public void testRenamePerformance() throws Exception {