snvijaya commented on a change in pull request #2021:
URL: https://github.com/apache/hadoop/pull/2021#discussion_r424958799
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java
##########
@@ -81,5 +81,7 @@
public static final String DEFAULT_FS_AZURE_USER_AGENT_PREFIX = EMPTY_STRING;
public static final String DEFAULT_VALUE_UNKNOWN = "UNKNOWN";
+ public static final boolean DEFAULT_DELETE_CONSIDERED_IDEMPOTENT = true;
Review comment:
DEFAULT prefix is to highlight the default behaviour of these fields in
ABFS driver. There do not mandate the need to be backed by user modifiable
config in AbfsConfiguration. AbfsConfiguration is the consumer of these
defaults.
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java
##########
@@ -28,20 +28,36 @@
import org.junit.Test;
+import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
+import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
+import org.apache.hadoop.fs.azurebfs.services.AbfsPerfTracker;
+import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
+import org.apache.hadoop.fs.azurebfs.services.ExponentialRetryPolicy;
+import org.apache.hadoop.fs.azurebfs.services.SharedKeyCredentials;
import org.apache.hadoop.fs.FileAlreadyExistsException;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DOT;
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_DELETE_CONSIDERED_IDEMPOTENT;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertDeleted;
import static
org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
/**
* Test delete operation.
*/
public class ITestAzureBlobFileSystemDelete extends
AbstractAbfsIntegrationTest {
+ private final int reducedRetryCount = 1;
Review comment:
Fixed
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java
##########
@@ -130,4 +146,49 @@ public Void call() throws Exception {
assertPathDoesNotExist(fs, "deleted", dir);
}
+
+ @Test
+ public void testDeleteIdempotency() throws Exception {
+ org.junit.Assume.assumeTrue(DEFAULT_DELETE_CONSIDERED_IDEMPOTENT);
+ // Config to reduce the retry and maxBackoff time for test run
+ AbfsConfiguration abfsConfig = getConfiguration();
+ abfsConfig.setMaxIoRetries(reducedRetryCount);
+ abfsConfig.setMaxBackoffIntervalMilliseconds(reducedMaxBackoffIntervalMs);
+
+ final AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient abfsClient = fs.getAbfsStore().getClient();
+ AbfsPerfTracker tracker = new AbfsPerfTracker("test",
+ this.getAccountName(),
+ abfsConfig);
+
+ // Create test AbfsClient
+ AbfsClient testClient = new AbfsClient(
+ abfsClient.getBaseUrl(),
+ new SharedKeyCredentials(abfsConfig.getAccountName().substring(0,
+ abfsConfig.getAccountName().indexOf(DOT)),
+ abfsConfig.getStorageAccountKey()),
+ abfsConfig,
+ new ExponentialRetryPolicy(reducedRetryCount),
+ abfsConfig.getTokenProvider(),
+ tracker);
+
+ // Mock instance of AbfsRestOperation
+ AbfsRestOperation op = mock(AbfsRestOperation.class);
+ // Set retryCount to non-zero
+ when(op.getRetryCount()).thenReturn(reducedRetryCount);
+
+ // Mock instance of Http Operation response. This will return HTTP:Not
Found
+ AbfsHttpOperation http404Op = mock(AbfsHttpOperation.class);
+ when(http404Op.getStatusCode()).thenReturn(HTTP_NOT_FOUND);
+
+ // Mock delete response to 404
+ when(op.getResult()).thenReturn(http404Op);
+
+ assertTrue(
Review comment:
Fixed
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java
##########
@@ -28,20 +28,37 @@
import org.junit.Assert;
import org.junit.Test;
+import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
+import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
+import org.apache.hadoop.fs.azurebfs.services.AbfsPerfTracker;
+import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
+import org.apache.hadoop.fs.azurebfs.services.ExponentialRetryPolicy;
+import org.apache.hadoop.fs.azurebfs.services.SharedKeyCredentials;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
+import static java.util.UUID.randomUUID;
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DOT;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs;
import static
org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
import static
org.apache.hadoop.fs.contract.ContractTestUtils.assertRenameOutcome;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile;
+import static org.mockito.Mockito.mock;
Review comment:
Fixed
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java
##########
@@ -28,20 +28,37 @@
import org.junit.Assert;
import org.junit.Test;
+import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
+import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
+import org.apache.hadoop.fs.azurebfs.services.AbfsPerfTracker;
+import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
+import org.apache.hadoop.fs.azurebfs.services.ExponentialRetryPolicy;
+import org.apache.hadoop.fs.azurebfs.services.SharedKeyCredentials;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
+import static java.util.UUID.randomUUID;
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DOT;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs;
import static
org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
import static
org.apache.hadoop.fs.contract.ContractTestUtils.assertRenameOutcome;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
/**
* Test rename operation.
*/
public class ITestAzureBlobFileSystemRename extends
AbstractAbfsIntegrationTest {
+ private final int reducedRetryCount = 1;
Review comment:
Fixed
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
