You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2017/08/18 13:38:24 UTC
[1/2] hadoop git commit: HADOOP-14769. WASB: delete recursive should
not fail if a file is deleted. Contributed by Thomas Marquardt
Repository: hadoop
Updated Branches:
refs/heads/branch-2 a5bc30170 -> c42e694ee
refs/heads/trunk 99e558b13 -> c6b4e656b
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/trunk
Commit: c6b4e656b76b68cc1d0dbcc15a5aa5ea23335b7b
Parents: 99e558b
Author: Steve Loughran <st...@apache.org>
Authored: Fri Aug 18 14:13:40 2017 +0100
Committer: Steve Loughran <st...@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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[2/2] hadoop git commit: HADOOP-14769. WASB: delete recursive should
not fail if a file is deleted. Contributed by Thomas Marquardt
Posted by st...@apache.org.
HADOOP-14769. WASB: delete recursive should not fail if a file is deleted.
Contributed by Thomas Marquardt
(cherry picked from commit c6b4e656b76b68cc1d0dbcc15a5aa5ea23335b7b)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c42e694e
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c42e694e
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c42e694e
Branch: refs/heads/branch-2
Commit: c42e694ee5a52dacf8ef35f0dbcaa62f99905295
Parents: a5bc301
Author: Steve Loughran <st...@apache.org>
Authored: Fri Aug 18 14:38:04 2017 +0100
Committer: Steve Loughran <st...@apache.org>
Committed: Fri Aug 18 14:38:04 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/c42e694e/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 4f997f2..038d160 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/c42e694e/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 bf4d744..b0b872d 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
@@ -2041,7 +2041,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;
}
};
@@ -2078,30 +2083,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/c42e694e/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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org