HADOOP-14769. WASB: delete recursive should not fail if a file is deleted. Contributed by Thomas Marquardt
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c6b4e656 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c6b4e656 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c6b4e656 Branch: refs/heads/YARN-5972 Commit: c6b4e656b76b68cc1d0dbcc15a5aa5ea23335b7b Parents: 99e558b Author: Steve Loughran <ste...@apache.org> Authored: Fri Aug 18 14:13:40 2017 +0100 Committer: Steve Loughran <ste...@apache.org> Committed: Fri Aug 18 14:13:40 2017 +0100 ---------------------------------------------------------------------- .../fs/azure/AzureNativeFileSystemStore.java | 21 ++++--- .../hadoop/fs/azure/NativeAzureFileSystem.java | 47 ++++++++------- .../TestFileSystemOperationsWithThreads.java | 61 ++++++++++++++++---- 3 files changed, 86 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/c6b4e656/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java index 554027b..b0cd701 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java @@ -2459,8 +2459,11 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore { try { blob.delete(operationContext, lease); } catch (StorageException e) { - LOG.error("Encountered Storage Exception for delete on Blob: {}, Exception Details: {} Error Code: {}", - blob.getUri(), e.getMessage(), e.getErrorCode()); + if (!NativeAzureFileSystemHelper.isFileNotFoundException(e)) { + LOG.error("Encountered Storage Exception for delete on Blob: {}" + + ", Exception Details: {} Error Code: {}", + blob.getUri(), e.getMessage(), e.getErrorCode()); + } // On exception, check that if: // 1. It's a BlobNotFound exception AND // 2. It got there after one-or-more retries THEN @@ -2491,17 +2494,17 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore { // Container doesn't exist, no need to do anything return true; } - // Get the blob reference and delete it. CloudBlobWrapper blob = getBlobReference(key); - if (blob.exists(getInstrumentedContext())) { - safeDelete(blob, lease); - return true; - } else { + safeDelete(blob, lease); + return true; + } catch (Exception e) { + if (e instanceof StorageException + && NativeAzureFileSystemHelper.isFileNotFoundException( + (StorageException) e)) { + // the file or directory does not exist return false; } - } catch (Exception e) { - // Re-throw as an Azure storage exception. throw new AzureException(e); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/c6b4e656/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java index a7558a3..2abc6c6 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java @@ -2043,7 +2043,12 @@ public class NativeAzureFileSystem extends FileSystem { AzureFileSystemThreadTask task = new AzureFileSystemThreadTask() { @Override public boolean execute(FileMetadata file) throws IOException{ - return deleteFile(file.getKey(), file.isDir()); + if (!deleteFile(file.getKey(), file.isDir())) { + LOG.warn("Attempt to delete non-existent {} {}", + file.isDir() ? "directory" : "file", + file.getKey()); + } + return true; } }; @@ -2080,30 +2085,28 @@ public class NativeAzureFileSystem extends FileSystem { return new AzureFileSystemThreadPoolExecutor(threadCount, threadNamePrefix, operation, key, config); } - // Delete single file / directory from key. + /** + * Delete the specified file or directory and increment metrics. + * If the file or directory does not exist, the operation returns false. + * @param path the path to a file or directory. + * @param isDir true if the path is a directory; otherwise false. + * @return true if delete is successful; otherwise false. + * @throws IOException if an IO error occurs while attempting to delete the + * path. + * + */ @VisibleForTesting - boolean deleteFile(String key, boolean isDir) throws IOException { - try { - if (store.delete(key)) { - if (isDir) { - instrumentation.directoryDeleted(); - } else { - instrumentation.fileDeleted(); - } - return true; - } else { - return false; - } - } catch(IOException e) { - Throwable innerException = NativeAzureFileSystemHelper.checkForAzureStorageException(e); - - if (innerException instanceof StorageException - && NativeAzureFileSystemHelper.isFileNotFoundException((StorageException) innerException)) { - return false; - } + boolean deleteFile(String path, boolean isDir) throws IOException { + if (!store.delete(path)) { + return false; + } - throw e; + if (isDir) { + instrumentation.directoryDeleted(); + } else { + instrumentation.fileDeleted(); } + return true; } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/c6b4e656/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsWithThreads.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsWithThreads.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsWithThreads.java index ce3cdee..fd3690c 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsWithThreads.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsWithThreads.java @@ -39,6 +39,8 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; /** * Tests the Native Azure file system (WASB) using parallel threads for rename and delete operations. @@ -529,30 +531,65 @@ public class TestFileSystemOperationsWithThreads extends AbstractWasbTestBase { } /* - * Test case for delete operation with multiple threads and flat listing enabled. + * Validate that when a directory is deleted recursively, the operation succeeds + * even if a child directory delete fails because the directory does not exist. + * This can happen if a child directory is deleted by an external agent while + * the parent is in progress of being deleted recursively. + */ + @Test + public void testRecursiveDirectoryDeleteWhenChildDirectoryDeleted() + throws Exception { + testRecusiveDirectoryDelete(true); + } + + /* + * Validate that when a directory is deleted recursively, the operation succeeds + * even if a file delete fails because it does not exist. + * This can happen if a file is deleted by an external agent while + * the parent directory is in progress of being deleted. */ @Test - public void testDeleteSingleDeleteFailure() throws Exception { + public void testRecursiveDirectoryDeleteWhenDeletingChildFileReturnsFalse() + throws Exception { + testRecusiveDirectoryDelete(false); + } + private void testRecusiveDirectoryDelete(boolean useDir) throws Exception { + String childPathToBeDeletedByExternalAgent = (useDir) + ? "root/0" + : "root/0/fileToRename"; // Spy azure file system object and return false for deleting one file - LOG.info("testDeleteSingleDeleteFailure"); NativeAzureFileSystem mockFs = Mockito.spy((NativeAzureFileSystem) fs); - String path = mockFs.pathToKey(mockFs.makeAbsolute(new Path("root/0"))); - Mockito.when(mockFs.deleteFile(path, true)).thenReturn(false); + String path = mockFs.pathToKey(mockFs.makeAbsolute(new Path( + childPathToBeDeletedByExternalAgent))); + + Answer<Boolean> answer = new Answer<Boolean>() { + public Boolean answer(InvocationOnMock invocation) throws Throwable { + String path = (String) invocation.getArguments()[0]; + boolean isDir = (boolean) invocation.getArguments()[1]; + boolean realResult = fs.deleteFile(path, isDir); + assertTrue(realResult); + boolean fakeResult = false; + return fakeResult; + } + }; + + Mockito.when(mockFs.deleteFile(path, useDir)).thenAnswer(answer); createFolder(mockFs, "root"); Path sourceFolder = new Path("root"); - assertFalse(mockFs.delete(sourceFolder, true)); - assertTrue(mockFs.exists(sourceFolder)); - // Validate from logs that threads are enabled and delete operation failed. + assertTrue(mockFs.delete(sourceFolder, true)); + assertFalse(mockFs.exists(sourceFolder)); + + // Validate from logs that threads are enabled, that a child directory was + // deleted by an external caller, and the parent delete operation still + // succeeds. String content = logs.getOutput(); assertInLog(content, "Using thread pool for Delete operation with threads"); - assertInLog(content, "Delete operation failed for file " + path); - assertInLog(content, - "Terminating execution of Delete operation now as some other thread already got exception or operation failed"); - assertInLog(content, "Failed to delete files / subfolders in blob"); + assertInLog(content, String.format("Attempt to delete non-existent %s %s", + useDir ? "directory" : "file", path)); } /* --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
