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 2016/12/08 22:29:18 UTC
[11/50] [abbrv] hadoop git commit: HADOOP-13675. Bug in return value
for delete() calls in WASB. Contributed by Dushyanth
HADOOP-13675. Bug in return value for delete() calls in WASB. Contributed by Dushyanth
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/15dd1f33
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/15dd1f33
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/15dd1f33
Branch: refs/heads/HADOOP-13345
Commit: 15dd1f3381069c5fdc6690e3ab1907a133ba14bf
Parents: 8c46808
Author: Mingliang Liu <li...@apache.org>
Authored: Mon Dec 5 12:04:07 2016 -0800
Committer: Mingliang Liu <li...@apache.org>
Committed: Mon Dec 5 12:04:07 2016 -0800
----------------------------------------------------------------------
.../fs/azure/AzureNativeFileSystemStore.java | 31 +++--
.../hadoop/fs/azure/NativeAzureFileSystem.java | 25 ++--
.../hadoop/fs/azure/NativeFileSystemStore.java | 23 +++-
...estNativeAzureFileSystemConcurrencyLive.java | 119 +++++++++++++++++++
4 files changed, 176 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/15dd1f33/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 3e864a4..ac6c514 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
@@ -2045,10 +2045,10 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
* The key to search for.
* @return The wanted directory, or null if not found.
*/
- private static FileMetadata getDirectoryInList(
+ private static FileMetadata getFileMetadataInList(
final Iterable<FileMetadata> list, String key) {
for (FileMetadata current : list) {
- if (current.isDir() && current.getKey().equals(key)) {
+ if (current.getKey().equals(key)) {
return current;
}
}
@@ -2114,7 +2114,7 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
// Add the metadata to the list, but remove any existing duplicate
// entries first that we may have added by finding nested files.
- FileMetadata existing = getDirectoryInList(fileMetadata, blobKey);
+ FileMetadata existing = getFileMetadataInList(fileMetadata, blobKey);
if (existing != null) {
fileMetadata.remove(existing);
}
@@ -2141,7 +2141,7 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
// Add the directory metadata to the list only if it's not already
// there.
- if (getDirectoryInList(fileMetadata, dirKey) == null) {
+ if (getFileMetadataInList(fileMetadata, dirKey) == null) {
fileMetadata.add(directoryMetadata);
}
@@ -2249,7 +2249,7 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
// Add the directory metadata to the list only if it's not already
// there.
- FileMetadata existing = getDirectoryInList(aFileMetadataList, blobKey);
+ FileMetadata existing = getFileMetadataInList(aFileMetadataList, blobKey);
if (existing != null) {
aFileMetadataList.remove(existing);
}
@@ -2278,7 +2278,7 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
// absolute path is being used or not.
String dirKey = normalizeKey(directory);
- if (getDirectoryInList(aFileMetadataList, dirKey) == null) {
+ if (getFileMetadataInList(aFileMetadataList, dirKey) == null) {
// Reached the targeted listing depth. Return metadata for the
// directory using default permissions.
//
@@ -2376,18 +2376,24 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
}
}
+ /**
+ * API implementation to delete a blob in the back end azure storage.
+ */
@Override
- public void delete(String key, SelfRenewingLease lease) throws IOException {
+ public boolean delete(String key, SelfRenewingLease lease) throws IOException {
try {
if (checkContainer(ContainerAccessType.ReadThenWrite) == ContainerState.DoesntExist) {
// Container doesn't exist, no need to do anything
- return;
+ return true;
}
// Get the blob reference and delete it.
CloudBlobWrapper blob = getBlobReference(key);
if (blob.exists(getInstrumentedContext())) {
safeDelete(blob, lease);
+ return true;
+ } else {
+ return false;
}
} catch (Exception e) {
// Re-throw as an Azure storage exception.
@@ -2395,10 +2401,13 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
}
}
+ /**
+ * API implementation to delete a blob in the back end azure storage.
+ */
@Override
- public void delete(String key) throws IOException {
+ public boolean delete(String key) throws IOException {
try {
- delete(key, null);
+ return delete(key, null);
} catch (IOException e) {
Throwable t = e.getCause();
if(t != null && t instanceof StorageException) {
@@ -2407,7 +2416,7 @@ public class AzureNativeFileSystemStore implements NativeFileSystemStore {
SelfRenewingLease lease = null;
try {
lease = acquireLease(key);
- delete(key, lease);
+ return delete(key, lease);
} catch (AzureException e3) {
LOG.warn("Got unexpected exception trying to acquire lease on "
+ key + "." + e3.getMessage());
http://git-wip-us.apache.org/repos/asf/hadoop/blob/15dd1f33/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 54eb90f..307c2fa 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
@@ -1765,8 +1765,11 @@ public class NativeAzureFileSystem extends FileSystem {
}
try {
- store.delete(key);
- instrumentation.fileDeleted();
+ if (store.delete(key)) {
+ instrumentation.fileDeleted();
+ } else {
+ return false;
+ }
} catch(IOException e) {
Throwable innerException = NativeAzureFileSystemHelper.checkForAzureStorageException(e);
@@ -1885,7 +1888,7 @@ public class NativeAzureFileSystem extends FileSystem {
}
// Delete the current directory
- if (!deleteFile(metaFile.getKey(), metaFile.isDir())) {
+ if (store.retrieveMetadata(metaFile.getKey()) != null && !deleteFile(metaFile.getKey(), metaFile.isDir())) {
LOG.error("Failed delete directory {}", f.toString());
return false;
}
@@ -1913,11 +1916,15 @@ public class NativeAzureFileSystem extends FileSystem {
@VisibleForTesting
boolean deleteFile(String key, boolean isDir) throws IOException {
try {
- store.delete(key);
- if (isDir) {
- instrumentation.directoryDeleted();
+ if (store.delete(key)) {
+ if (isDir) {
+ instrumentation.directoryDeleted();
+ } else {
+ instrumentation.fileDeleted();
+ }
+ return true;
} else {
- instrumentation.fileDeleted();
+ return false;
}
} catch(IOException e) {
Throwable innerException = NativeAzureFileSystemHelper.checkForAzureStorageException(e);
@@ -1929,8 +1936,6 @@ public class NativeAzureFileSystem extends FileSystem {
throw e;
}
-
- return true;
}
@Override
@@ -2790,6 +2795,8 @@ public class NativeAzureFileSystem extends FileSystem {
throws IOException {
LOG.debug("Deleting dangling file {}", file.getKey());
+ // Not handling delete return type as false return essentially
+ // means its a no-op for the caller
store.delete(file.getKey());
store.delete(tempFile.getKey());
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/15dd1f33/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeFileSystemStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeFileSystemStore.java
index acdd3d6..454a5df 100644
--- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeFileSystemStore.java
+++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeFileSystemStore.java
@@ -74,7 +74,16 @@ interface NativeFileSystemStore {
void changePermissionStatus(String key, PermissionStatus newPermission)
throws AzureException;
- void delete(String key) throws IOException;
+ /**
+ * API to delete a blob in the back end azure storage.
+ * @param key - key to the blob being deleted.
+ * @return return true when delete is successful, false if
+ * blob cannot be found or delete is not possible without
+ * exception.
+ * @throws IOException Exception encountered while deleting in
+ * azure storage.
+ */
+ boolean delete(String key) throws IOException;
void rename(String srcKey, String dstKey) throws IOException;
@@ -104,7 +113,17 @@ interface NativeFileSystemStore {
void updateFolderLastModifiedTime(String key, Date lastModified,
SelfRenewingLease folderLease) throws AzureException;
- void delete(String key, SelfRenewingLease lease) throws IOException;
+ /**
+ * API to delete a blob in the back end azure storage.
+ * @param key - key to the blob being deleted.
+ * @param lease - Active lease on the blob.
+ * @return return true when delete is successful, false if
+ * blob cannot be found or delete is not possible without
+ * exception.
+ * @throws IOException Exception encountered while deleting in
+ * azure storage.
+ */
+ boolean delete(String key, SelfRenewingLease lease) throws IOException;
SelfRenewingLease acquireLease(String key) throws AzureException;
http://git-wip-us.apache.org/repos/asf/hadoop/blob/15dd1f33/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemConcurrencyLive.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemConcurrencyLive.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemConcurrencyLive.java
new file mode 100644
index 0000000..ec72cce
--- /dev/null
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemConcurrencyLive.java
@@ -0,0 +1,119 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azure;
+
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Assert;
+import org.junit.Test;
+
+/***
+ * Test class to hold all Live Azure storage concurrency tests.
+ */
+public class TestNativeAzureFileSystemConcurrencyLive
+ extends AbstractWasbTestBase {
+
+ private static final int TEST_COUNT = 102;
+ @Override
+ protected AzureBlobStorageTestAccount createTestAccount() throws Exception {
+ return AzureBlobStorageTestAccount.create();
+ }
+
+ /**
+ * Test multi-threaded deletes in WASB. Expected behavior is one of the thread
+ * should be to successfully delete the file and return true and all other
+ * threads need to return false.
+ */
+ @Test
+ public void testMultiThreadedDeletes() throws Exception {
+ Path testFile = new Path("test.dat");
+ fs.create(testFile).close();
+
+ int threadCount = TEST_COUNT;
+ DeleteHelperThread[] helperThreads = new DeleteHelperThread[threadCount];
+
+ for (int i = 0; i < threadCount; i++) {
+ helperThreads[i] = new DeleteHelperThread(fs, testFile);
+ }
+
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threadCount; i++) {
+ threads[i] = new Thread(helperThreads[i]);
+ threads[i].start();
+ }
+
+ for (int i = 0; i < threadCount; i++) {
+ threads[i].join();
+ }
+
+ boolean deleteSuccess = false;
+
+ for (int i = 0; i < threadCount; i++) {
+
+ Assert.assertFalse("child thread has exception : " + helperThreads[i].getException(),
+ helperThreads[i].getExceptionEncounteredFlag());
+
+ if (deleteSuccess) {
+ Assert.assertFalse("More than one thread delete() retuhelperThreads[i].getDeleteSuccess()",
+ helperThreads[i].getExceptionEncounteredFlag());
+ } else {
+ deleteSuccess = helperThreads[i].getDeleteSuccess();
+ }
+ }
+
+ Assert.assertTrue("No successfull delete found", deleteSuccess);
+ }
+}
+
+class DeleteHelperThread implements Runnable {
+
+ private FileSystem fs;
+ private Path p;
+ private boolean deleteSuccess;
+ private boolean exceptionEncountered;
+ private Exception ex;
+
+ public DeleteHelperThread(FileSystem fs, Path p) {
+ this.fs = fs;
+ this.p = p;
+ }
+
+ public void run() {
+ try {
+ deleteSuccess = fs.delete(p, false);
+ } catch (Exception ioEx) {
+ exceptionEncountered = true;
+ this.ex = ioEx;
+ }
+ }
+
+ public boolean getDeleteSuccess() {
+ return deleteSuccess;
+ }
+
+ public boolean getExceptionEncounteredFlag() {
+ return exceptionEncountered;
+ }
+
+ public Exception getException() {
+ return ex;
+ }
+}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org