From 5a11d4d5c8edd1feb9260afddd8ed8319f0347de Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Wed, 29 Apr 2020 22:05:12 +0530 Subject: [PATCH 01/11] rename changes --- .../fs/azurebfs/services/AbfsClient.java | 32 +++++++++++++++++++ .../azurebfs/services/AbfsRestOperation.java | 7 ++++ 2 files changed, 39 insertions(+) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 6bacde48668e2..9561dac5a9055 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -21,6 +21,7 @@ import java.io.Closeable; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.net.URLEncoder; @@ -321,6 +322,21 @@ public AbfsRestOperation renamePath(String source, final String destination, fin url, requestHeaders); op.execute(); + + if ((op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && + op.isARetriedRequest()) { + // Driver retried the rename request + // the first failing request might have succeeded at server. + // Server has returned HTTP 404, which means rename source no longer + // exists. Check if destination path is present and return success. + + final AbfsRestOperation destStatusOp = getPathStatus(destination); + if (destStatusOp.getResult().getStatusCode() == HttpURLConnection.HTTP_OK) { + return destStatusOp; + } + + } + return op; } @@ -459,6 +475,22 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, url, requestHeaders); op.execute(); + + if ((op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && + op.isARetriedRequest()) { + // Driver retried the delete request + // the first failing request might have succeeded at server. + // Server has returned HTTP 404, which means path no longer + // exists, return success. + final AbfsRestOperation successOp = new AbfsRestOperation( + AbfsRestOperationType.DeletePath, + this, + HTTP_METHOD_DELETE, + url, + requestHeaders); + + } + return op; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java index 445c3665437c7..be02e5c92e27e 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java @@ -61,12 +61,18 @@ public class AbfsRestOperation { private int bufferOffset; private int bufferLength; + private boolean isARetriedRequest = false; + private AbfsHttpOperation result; public AbfsHttpOperation getResult() { return result; } + public boolean isARetriedRequest() { + return isARetriedRequest; + } + /** * Initializes a new REST operation. * @@ -133,6 +139,7 @@ void execute() throws AzureBlobFileSystemException { LOG.debug("First execution of REST operation - {}", operationType); while (!executeHttpOperation(retryCount++)) { try { + isARetriedRequest = true; LOG.debug("Retrying REST operation {}. RetryCount = {}", operationType, retryCount); Thread.sleep(client.getRetryPolicy().getRetryInterval(retryCount)); From 6471661ab59e297c8f476bf6ece229b8cc5a4bcf Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Wed, 13 May 2020 09:09:44 +0530 Subject: [PATCH 02/11] Some refactoring --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 5 + .../fs/azurebfs/AzureBlobFileSystemStore.java | 19 +-- .../constants/FileSystemConfigurations.java | 2 + .../fs/azurebfs/services/AbfsClient.java | 118 ++++++++++++-- .../azurebfs/services/AbfsHttpOperation.java | 13 ++ .../azurebfs/services/AbfsRestOperation.java | 23 ++- .../fs/azurebfs/utils/DateTimeUtils.java | 44 ++++++ .../ITestAzureBlobFileSystemDelete.java | 61 ++++++++ .../ITestAzureBlobFileSystemRename.java | 148 ++++++++++++++++++ 9 files changed, 395 insertions(+), 38 deletions(-) create mode 100644 hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index d60bc37734b8e..2b116a8ce14b4 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -755,6 +755,11 @@ public void setMaxIoRetries(int maxIoRetries) { this.maxIoRetries = maxIoRetries; } + @VisibleForTesting + public void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) { + this.maxBackoffInterval = maxBackoffInterval; + } + @VisibleForTesting void setIsNamespaceEnabledAccount(String isNamespaceEnabledAccount) { this.isNamespaceEnabledAccount = isNamespaceEnabledAccount; diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java index d37ceb3d5b57c..820057799c03d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java @@ -34,7 +34,6 @@ import java.nio.charset.CharsetDecoder; import java.nio.charset.CharsetEncoder; import java.nio.charset.StandardCharsets; -import java.text.ParseException; import java.text.SimpleDateFormat; import java.time.Instant; import java.util.ArrayList; @@ -96,6 +95,7 @@ import org.apache.hadoop.fs.azurebfs.services.AbfsPerfInfo; import org.apache.hadoop.fs.azurebfs.utils.Base64; import org.apache.hadoop.fs.azurebfs.utils.CRC64; +import org.apache.hadoop.fs.azurebfs.utils.DateTimeUtils; import org.apache.hadoop.fs.azurebfs.utils.UriUtils; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.AclStatus; @@ -128,7 +128,6 @@ public class AzureBlobFileSystemStore implements Closeable { private URI uri; private String userName; private String primaryUserGroup; - private static final String DATE_TIME_PATTERN = "E, dd MMM yyyy HH:mm:ss z"; private static final String TOKEN_DATE_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSSSSSS'Z'"; private static final String XMS_PROPERTIES_ENCODING = "ISO-8859-1"; private static final int GET_SET_AGGREGATE_COUNT = 2; @@ -662,7 +661,7 @@ public FileStatus getFileStatus(final Path path) throws IOException { resourceIsDir, 1, blockSize, - parseLastModifiedTime(lastModified), + DateTimeUtils.ParseLastModifiedTime(lastModified), path, eTag); } @@ -738,7 +737,7 @@ public FileStatus[] listStatus(final Path path, final String startFrom) throws I long contentLength = entry.contentLength() == null ? 0 : entry.contentLength(); boolean isDirectory = entry.isDirectory() == null ? false : entry.isDirectory(); if (entry.lastModified() != null && !entry.lastModified().isEmpty()) { - lastModifiedMillis = parseLastModifiedTime(entry.lastModified()); + lastModifiedMillis = DateTimeUtils.ParseLastModifiedTime(entry.lastModified()); } Path entryPath = new Path(File.separator + entry.name()); @@ -1232,18 +1231,6 @@ private boolean parseIsDirectory(final String resourceType) { && resourceType.equalsIgnoreCase(AbfsHttpConstants.DIRECTORY); } - private long parseLastModifiedTime(final String lastModifiedTime) { - long parsedTime = 0; - try { - Date utcDate = new SimpleDateFormat(DATE_TIME_PATTERN, Locale.US).parse(lastModifiedTime); - parsedTime = utcDate.getTime(); - } catch (ParseException e) { - LOG.error("Failed to parse the date {}", lastModifiedTime); - } finally { - return parsedTime; - } - } - private String convertXmsPropertiesToCommaSeparatedString(final Hashtable properties) throws CharacterCodingException { StringBuilder commaSeparatedProperties = new StringBuilder(); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java index ef2e708d5ad54..d20cda34ac2b3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java @@ -80,5 +80,7 @@ public final class FileSystemConfigurations { 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; + private FileSystemConfigurations() {} } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 9561dac5a9055..4cdb1f11a11d3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -34,6 +34,7 @@ import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; import org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,9 +45,11 @@ import org.apache.hadoop.fs.azurebfs.extensions.SASTokenProvider; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider; +import org.apache.hadoop.fs.azurebfs.utils.DateTimeUtils; import org.apache.hadoop.io.IOUtils; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.*; +import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_DELETE_CONSIDERED_IDEMPOTENT; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemUriSchemes.HTTPS_SCHEME; import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.*; import static org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams.*; @@ -131,7 +134,7 @@ public String getFileSystem() { return filesystem; } - protected AbfsPerfTracker getAbfsPerfTracker() { + public AbfsPerfTracker getAbfsPerfTracker() { return abfsPerfTracker; } @@ -323,23 +326,79 @@ public AbfsRestOperation renamePath(String source, final String destination, fin requestHeaders); op.execute(); - if ((op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && - op.isARetriedRequest()) { - // Driver retried the rename request - // the first failing request might have succeeded at server. + if (op.getResult().getStatusCode() != HttpURLConnection.HTTP_OK) { + return renameIdempotencyCheckOp(op, destination); + } + + return op; + } + + /** + * Check if the rename request failure is post a retry and if earlier rename + * request might have succeeded at back-end. + * + * If there is a parallel rename activity happening from any other store + * interface, the logic here will detect the rename to have happened due to + * the one initiated from this ABFS filesytem instance as it was retried. This + * should be a corner case hence going ahead with LMT check. + * @param op Rename request REST operation response + * @param destination rename destination path + * @return REST operation response post idempotency check + * @throws AzureBlobFileSystemException + */ + public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, + final String destination) throws AzureBlobFileSystemException { + if ((op.getRetryCount() > 0) && + (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) { // Server has returned HTTP 404, which means rename source no longer - // exists. Check if destination path is present and return success. + // exists. Check on destination status and if it has a recent LMT timestamp. + // If yes, return success, else fall back to original rename request failure response. final AbfsRestOperation destStatusOp = getPathStatus(destination); if (destStatusOp.getResult().getStatusCode() == HttpURLConnection.HTTP_OK) { - return destStatusOp; - } + String lmt = destStatusOp.getResult().getResponseHeader( + HttpHeaderConfigurations.LAST_MODIFIED); + if (isRecentlyModified(lmt, + getTimeIntervalToTagOperationRecent(op.getRetryCount()))) { + return destStatusOp; + } + } } return op; } + /** + * For operations that return user-error on retry because earlier operation + * had already succeeded on server end, max timespan for the operation to have + * updated file/folder LMT should be within retryCount * max wait time for + * each retry. To include factors like clock skew and request network time + * factors, setting the timespan to be 2 times the max timespan the + * re-tries could incurr. + * @return timespan in milli-seconds within which operation should have + * occurred to qualify as recent operation. + */ + public int getTimeIntervalToTagOperationRecent(int retryCount) { + return 2 * retryCount * abfsConfiguration.getMaxBackoffIntervalMilliseconds(); + } + + /** + * Tries to identify if an operation was recently executed based on the LMT of + * a file or folder. LMT is checked to be in a time interval to determine if it + * is a recent operation. + * @param lastModifiedTime File/Folder LMT + * @param timeIntervalToTagOperationRecent time interval in + * @return true if the LMT is within timespan for recent operation, else false + */ + private boolean isRecentlyModified(final String lastModifiedTime, + final int timeIntervalToTagOperationRecent) { + long lmtEpochTime = DateTimeUtils.ParseLastModifiedTime(lastModifiedTime); + long currentEpochTime = java.time.Instant.now().toEpochMilli(); + + return (currentEpochTime - lmtEpochTime) <= timeIntervalToTagOperationRecent; + } + public AbfsRestOperation append(final String path, final long position, final byte[] buffer, final int offset, final int length) throws AzureBlobFileSystemException { final List requestHeaders = createDefaultHeaders(); @@ -476,24 +535,49 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, requestHeaders); op.execute(); - if ((op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && - op.isARetriedRequest()) { - // Driver retried the delete request - // the first failing request might have succeeded at server. + if (op.getResult().getStatusCode() != HttpURLConnection.HTTP_OK) { + return deleteIdempotencyCheckOp(op); + } + + return op; + } + + /** + * Check if the delete request failure is post a retry and if delete failure + * qualifies to be a success response assuming idempotency. + * + * There are below scenarios where delete could be incorrectly deducted as + * success post request retry: + * 1. Target was originally not existing and initial delete request had to be + * re-tried. + * 2. Parallel delete issued from any other store interface rather than + * delete issued from this filesystem instance. + * These are few corner cases and usually returning a success at this stage + * should help the job to continue. + * @param op Delete request REST operation response + * @return REST operation response post idempotency check + * @throws AzureBlobFileSystemException + */ + public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) { + if ((op.getRetryCount() > 0) && + (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && + DEFAULT_DELETE_CONSIDERED_IDEMPOTENT) { // Server has returned HTTP 404, which means path no longer - // exists, return success. + // exists. Assuming delete result to be idempotent, return success. final AbfsRestOperation successOp = new AbfsRestOperation( AbfsRestOperationType.DeletePath, this, HTTP_METHOD_DELETE, - url, - requestHeaders); - + op.getUrl(), + op.getRequestHeaders()); + successOp.hardSetResult(HttpURLConnection.HTTP_OK); + return successOp; } return op; } + public AbfsRestOperation setOwner(final String path, final String owner, final String group) throws AzureBlobFileSystemException { final List requestHeaders = createDefaultHeaders(); @@ -768,7 +852,7 @@ private void appendIfNotEmpty(StringBuilder sb, String regEx, } @VisibleForTesting - URL getBaseUrl() { + public URL getBaseUrl() { return baseUrl; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java index 881d41f65f27d..2b59c2e9da7d3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java @@ -81,6 +81,19 @@ public class AbfsHttpOperation implements AbfsPerfLoggable { private long sendRequestTimeMs; private long recvResponseTimeMs; + public static AbfsHttpOperation GetAbfsHttpOperationWithFixedResult(final URL url, + final String method, final int httpStatus) { + return new AbfsHttpOperation(url, method, httpStatus); + } + + private AbfsHttpOperation(final URL url, final String method, + final int httpStatus) { + this.isTraceEnabled = LOG.isTraceEnabled(); + this.url = url; + this.method = method; + this.statusCode = httpStatus; + } + protected HttpURLConnection getConnection() { return connection; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java index be02e5c92e27e..70346f80b2f44 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java @@ -44,6 +44,7 @@ public class AbfsRestOperation { private final AbfsClient client; // the HTTP method (PUT, PATCH, POST, GET, HEAD, or DELETE) private final String method; + // full URL including query parameters private final URL url; // all the custom HTTP request headers provided by the caller @@ -61,7 +62,7 @@ public class AbfsRestOperation { private int bufferOffset; private int bufferLength; - private boolean isARetriedRequest = false; + private int retryCount = 0; private AbfsHttpOperation result; @@ -69,8 +70,21 @@ public AbfsHttpOperation getResult() { return result; } - public boolean isARetriedRequest() { - return isARetriedRequest; + public void hardSetResult(int httpStatus) { + result = AbfsHttpOperation.GetAbfsHttpOperationWithFixedResult(this.url, + this.method, httpStatus); + } + + public URL getUrl() { + return url; + } + + public List getRequestHeaders() { + return requestHeaders; + } + + public int getRetryCount() { + return retryCount; } /** @@ -135,11 +149,10 @@ void execute() throws AzureBlobFileSystemException { requestHeaders.add(httpHeader); } - int retryCount = 0; + retryCount = 0; LOG.debug("First execution of REST operation - {}", operationType); while (!executeHttpOperation(retryCount++)) { try { - isARetriedRequest = true; LOG.debug("Retrying REST operation {}. RetryCount = {}", operationType, retryCount); Thread.sleep(client.getRetryPolicy().getRetryInterval(retryCount)); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java new file mode 100644 index 0000000000000..377d1400f37fe --- /dev/null +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java @@ -0,0 +1,44 @@ +/** + * 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.azurebfs.utils; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.Locale; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class DateTimeUtils { + public static final Logger LOG = LoggerFactory.getLogger(DateTimeUtils.class); + private static final String DATE_TIME_PATTERN = "E, dd MMM yyyy HH:mm:ss z"; + + public static long ParseLastModifiedTime(final String lastModifiedTime) { + long parsedTime = 0; + try { + Date utcDate = new SimpleDateFormat(DATE_TIME_PATTERN, Locale.US).parse(lastModifiedTime); + parsedTime = utcDate.getTime(); + } catch (ParseException e) { + LOG.error("Failed to parse the date {}", lastModifiedTime); + } finally { + return parsedTime; + } + } +} diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index 486daca4f1120..d51fdd84fab2c 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -28,13 +28,26 @@ 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. @@ -42,6 +55,9 @@ public class ITestAzureBlobFileSystemDelete extends AbstractAbfsIntegrationTest { + private final int reducedRetryCount = 1; + private final int reducedMaxBackoffIntervalMs = 5000; + public ITestAzureBlobFileSystemDelete() throws Exception { super(); } @@ -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( + "Delete is considered idempotent by default and should return success.", + (testClient.deleteIdempotencyCheckOp(op) + .getResult() + .getStatusCode() == HTTP_OK)); + } + } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index e0e1d899a2184..76fd99b4a0a8b 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -28,13 +28,27 @@ 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. @@ -42,6 +56,9 @@ public class ITestAzureBlobFileSystemRename extends AbstractAbfsIntegrationTest { + private final int reducedRetryCount = 1; + private final int reducedMaxBackoffIntervalMs = 5000; + public ITestAzureBlobFileSystemRename() throws Exception { super(); } @@ -149,4 +166,135 @@ public void testPosixRenameDirectory() throws Exception { assertTrue(fs.exists(new Path("testDir2/test4/test3"))); assertFalse(fs.exists(new Path("testDir2/test1/test2/test3"))); } + + @Test + public void testRenameRetryFailureAsHTTP400() throws Exception { + // Rename failed as Bad Request + // RenameIdempotencyCheck should throw back the rename failure Op + testRenameTimeout(HTTP_BAD_REQUEST, HTTP_BAD_REQUEST, false); + } + + @Test + public void testRenameRetryFailureAsHTTP404() throws Exception { + // Rename failed as FileNotFound and the destination LMT is + // within TimespanForIdentifyingRecentOperationThroughLMT + testRenameTimeout(HTTP_NOT_FOUND, HTTP_OK, false); + } + + @Test + public void testRenameRetryFailureWithDestOldLMT() throws Exception { + // Rename failed as FileNotFound and the destination LMT is + // older than TimespanForIdentifyingRecentOperationThroughLMT + testRenameTimeout(HTTP_NOT_FOUND, HTTP_NOT_FOUND, true); + } + + private void testRenameTimeout( + int renameRequestStatus, + int renameIdempotencyCheckStatus, + boolean isOldOp) throws Exception { + // 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); + + // Timespan for which LMT will be considered recent. + assertTrue( + "Timestamp range for LMT update should be twice of " + + "MaxIoRetries * MaxBackoffIntervalMilliseconds", + testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount) == + (2 * reducedRetryCount * reducedMaxBackoffIntervalMs)); + + // 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:Bad Request + AbfsHttpOperation http400Op = mock(AbfsHttpOperation.class); + when(http400Op.getStatusCode()).thenReturn(HTTP_BAD_REQUEST); + + // Mock instance of Http Operation response. This will return HTTP:Not Found + AbfsHttpOperation http404Op = mock(AbfsHttpOperation.class); + when(http404Op.getStatusCode()).thenReturn(HTTP_NOT_FOUND); + + Path destinationPath = fs.makeQualified( + new Path("destination" + randomUUID().toString())); + + if (renameRequestStatus == HTTP_BAD_REQUEST) { + // case 1: Rename failed as Bad Request + // RenameIdempotencyCheck should throw back the rename failure Op + + // Mock response + // passed in op.getResult() will be called twice. Within renameIdempotencyCheckOp, + // and as the return value from renameIdempotencyCheckOp. + // In both cases return 400 Bad Rquest response. + when(op.getResult()).thenReturn(http400Op); + + assertTrue( + "renameIdempotencyCheckOp should return rename BadRequest response itself.", + (testClient.renameIdempotencyCheckOp(op, destinationPath.toUri().getPath()) + .getResult() + .getStatusCode() == renameIdempotencyCheckStatus)); + } else if (isOldOp == false) { + // case 2: Rename failed as FileNotFound and the destination LMT is + // within TimespanForIdentifyingRecentOperationThroughLMT + + // Mock response + // passed in op.getResult() will be called only once. Within renameIdempotencyCheckOp, + // as return will be of GetFileStatus Op which should be 200 OK. + when(op.getResult()).thenReturn(http404Op); + + // Create the file new. + fs.create(destinationPath); + + assertTrue( + "Rename should return success response because the destination " + + "path is present and its LMT is within TimespanForIdentifyingRecentOperationThroughLMT.", + (testClient.renameIdempotencyCheckOp(op, destinationPath.toUri().getPath()) + .getResult() + .getStatusCode() == renameIdempotencyCheckStatus)); + } else { + // case 3: Rename failed as FileNotFound and the destination LMT is + // older than TimespanForIdentifyingRecentOperationThroughLMT + + // Create the file new. + fs.create(destinationPath); + + // sleep to cross the threshold for old LMT. + Thread.sleep( + testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount)); + + // Mock response + // passed in op.getResult() will be called twice. Within renameIdempotencyCheckOp, + // as return Op. Though GetFileStatus will be called, being an older LMT, + // rename op response will be returned. + when(op.getResult()).thenReturn(http404Op); + + assertTrue( + "Rename should return original rename failure response because the destination " + + "path LMT is older than TimespanForIdentifyingRecentOperationThroughLMT.", + (testClient.renameIdempotencyCheckOp(op, + destinationPath.toUri().getPath()) + .getResult() + .getStatusCode() == renameIdempotencyCheckStatus)); + } + + } } From c158597fa2efc7e75cbfd09ea3ff48b0e0b8904a Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Wed, 13 May 2020 21:53:47 +0530 Subject: [PATCH 03/11] Checkstyle and findbugs fixes --- .../fs/azurebfs/AzureBlobFileSystemStore.java | 4 ++-- .../hadoop/fs/azurebfs/services/AbfsClient.java | 16 ++++++++-------- .../fs/azurebfs/services/AbfsHttpOperation.java | 2 +- .../fs/azurebfs/services/AbfsRestOperation.java | 2 +- .../hadoop/fs/azurebfs/utils/DateTimeUtils.java | 6 +++++- .../hadoop-azure/src/site/markdown/abfs.md | 12 ++++++++++++ .../azurebfs/ITestAzureBlobFileSystemRename.java | 6 +++--- 7 files changed, 32 insertions(+), 16 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java index b03e225d33411..969ad691daaa8 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java @@ -671,7 +671,7 @@ public FileStatus getFileStatus(final Path path) throws IOException { resourceIsDir, 1, blockSize, - DateTimeUtils.ParseLastModifiedTime(lastModified), + DateTimeUtils.parseLastModifiedTime(lastModified), path, eTag); } @@ -747,7 +747,7 @@ public FileStatus[] listStatus(final Path path, final String startFrom) throws I long contentLength = entry.contentLength() == null ? 0 : entry.contentLength(); boolean isDirectory = entry.isDirectory() == null ? false : entry.isDirectory(); if (entry.lastModified() != null && !entry.lastModified().isEmpty()) { - lastModifiedMillis = DateTimeUtils.ParseLastModifiedTime(entry.lastModified()); + lastModifiedMillis = DateTimeUtils.parseLastModifiedTime(entry.lastModified()); } Path entryPath = new Path(File.separator + entry.name()); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index de489dfa0d523..2df8953670110 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -344,12 +344,12 @@ public AbfsRestOperation renamePath(String source, final String destination, fin * @param op Rename request REST operation response * @param destination rename destination path * @return REST operation response post idempotency check - * @throws AzureBlobFileSystemException + * @throws AzureBlobFileSystemException if GetFileStatus hits any exception */ public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, final String destination) throws AzureBlobFileSystemException { - if ((op.getRetryCount() > 0) && - (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) { + if ((op.getRetryCount() > 0) + && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) { // Server has returned HTTP 404, which means rename source no longer // exists. Check on destination status and if it has a recent LMT timestamp. // If yes, return success, else fall back to original rename request failure response. @@ -376,6 +376,7 @@ public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, * each retry. To include factors like clock skew and request network time * factors, setting the timespan to be 2 times the max timespan the * re-tries could incurr. + * @param retryCount number of retries done for the request * @return timespan in milli-seconds within which operation should have * occurred to qualify as recent operation. */ @@ -393,7 +394,7 @@ public int getTimeIntervalToTagOperationRecent(int retryCount) { */ private boolean isRecentlyModified(final String lastModifiedTime, final int timeIntervalToTagOperationRecent) { - long lmtEpochTime = DateTimeUtils.ParseLastModifiedTime(lastModifiedTime); + long lmtEpochTime = DateTimeUtils.parseLastModifiedTime(lastModifiedTime); long currentEpochTime = java.time.Instant.now().toEpochMilli(); return (currentEpochTime - lmtEpochTime) <= timeIntervalToTagOperationRecent; @@ -573,12 +574,11 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, * should help the job to continue. * @param op Delete request REST operation response * @return REST operation response post idempotency check - * @throws AzureBlobFileSystemException */ public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) { - if ((op.getRetryCount() > 0) && - (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && - DEFAULT_DELETE_CONSIDERED_IDEMPOTENT) { + if ((op.getRetryCount() > 0) + && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) + && DEFAULT_DELETE_CONSIDERED_IDEMPOTENT) { // Server has returned HTTP 404, which means path no longer // exists. Assuming delete result to be idempotent, return success. final AbfsRestOperation successOp = new AbfsRestOperation( diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java index 2b59c2e9da7d3..5dc4a89a53cbc 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java @@ -81,7 +81,7 @@ public class AbfsHttpOperation implements AbfsPerfLoggable { private long sendRequestTimeMs; private long recvResponseTimeMs; - public static AbfsHttpOperation GetAbfsHttpOperationWithFixedResult(final URL url, + public static AbfsHttpOperation getAbfsHttpOperationWithFixedResult(final URL url, final String method, final int httpStatus) { return new AbfsHttpOperation(url, method, httpStatus); } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java index d1cb083bb48af..cca43a0881ffa 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java @@ -74,7 +74,7 @@ public AbfsHttpOperation getResult() { } public void hardSetResult(int httpStatus) { - result = AbfsHttpOperation.GetAbfsHttpOperationWithFixedResult(this.url, + result = AbfsHttpOperation.getAbfsHttpOperationWithFixedResult(this.url, this.method, httpStatus); } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java index 377d1400f37fe..72deb62111a75 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java @@ -30,7 +30,7 @@ public final class DateTimeUtils { public static final Logger LOG = LoggerFactory.getLogger(DateTimeUtils.class); private static final String DATE_TIME_PATTERN = "E, dd MMM yyyy HH:mm:ss z"; - public static long ParseLastModifiedTime(final String lastModifiedTime) { + public static long parseLastModifiedTime(final String lastModifiedTime) { long parsedTime = 0; try { Date utcDate = new SimpleDateFormat(DATE_TIME_PATTERN, Locale.US).parse(lastModifiedTime); @@ -41,4 +41,8 @@ public static long ParseLastModifiedTime(final String lastModifiedTime) { return parsedTime; } } + + private DateTimeUtils() { + + } } diff --git a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md index 89f52e7e84d69..77eb753930de5 100644 --- a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md +++ b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md @@ -691,6 +691,18 @@ Config `fs.azure.account.hns.enabled` provides an option to specify whether Config `fs.azure.enable.check.access` needs to be set true to enable the AzureBlobFileSystem.access(). +### Operation Idempotency + +Requests failing due to server timeouts and network failures will be retried +exponentially. PUT/POST operations are idempotent and need no specific handling +except for Rename and Delete operations. + +Rename idempotency checks are made by ensuring the LastModifiedTime on destination +is recent if source path is found to be non-existent on retry. + +Delete is considered to be idempotent by default if the target does not exist on +retry. + ### Perf Options #### 1. HTTP Request Tracking Options diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 76fd99b4a0a8b..0aba3c3383c90 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -218,8 +218,8 @@ private void testRenameTimeout( assertTrue( "Timestamp range for LMT update should be twice of " + "MaxIoRetries * MaxBackoffIntervalMilliseconds", - testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount) == - (2 * reducedRetryCount * reducedMaxBackoffIntervalMs)); + testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount) + == (2 * reducedRetryCount * reducedMaxBackoffIntervalMs)); // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); @@ -252,7 +252,7 @@ private void testRenameTimeout( (testClient.renameIdempotencyCheckOp(op, destinationPath.toUri().getPath()) .getResult() .getStatusCode() == renameIdempotencyCheckStatus)); - } else if (isOldOp == false) { + } else if (!isOldOp) { // case 2: Rename failed as FileNotFound and the destination LMT is // within TimespanForIdentifyingRecentOperationThroughLMT From 77f54cb8323ef8137df430aff3e16caf6ab3497f Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Wed, 13 May 2020 22:02:58 +0530 Subject: [PATCH 04/11] Removing redundant new lines --- .../org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 2 -- .../apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java | 3 --- .../org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java | 1 - 3 files changed, 6 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 2df8953670110..e0b4156fc738c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -35,7 +35,6 @@ import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; import org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -594,7 +593,6 @@ public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) { return op; } - public AbfsRestOperation setOwner(final String path, final String owner, final String group) throws AzureBlobFileSystemException { final List requestHeaders = createDefaultHeaders(); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java index cca43a0881ffa..67eb4145c4291 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java @@ -44,7 +44,6 @@ public class AbfsRestOperation { private final AbfsClient client; // the HTTP method (PUT, PATCH, POST, GET, HEAD, or DELETE) private final String method; - // full URL including query parameters private final URL url; // all the custom HTTP request headers provided by the caller @@ -64,7 +63,6 @@ public class AbfsRestOperation { private byte[] buffer; private int bufferOffset; private int bufferLength; - private int retryCount = 0; private AbfsHttpOperation result; @@ -92,7 +90,6 @@ public int getRetryCount() { String getSasToken() { return sasToken; - } /** diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java index 72deb62111a75..2fa06134716a2 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java @@ -43,6 +43,5 @@ public static long parseLastModifiedTime(final String lastModifiedTime) { } private DateTimeUtils() { - } } From be7c0240a9507f4eb80f52f5a3f2c3f7899f249b Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Thu, 14 May 2020 12:54:59 +0530 Subject: [PATCH 05/11] Review comments --- .../fs/azurebfs/AzureBlobFileSystemStore.java | 3 +- .../fs/azurebfs/utils/DateTimeUtils.java | 5 +- .../ITestAzureBlobFileSystemDelete.java | 25 +++-- .../ITestAzureBlobFileSystemRename.java | 106 +++++++----------- 4 files changed, 61 insertions(+), 78 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java index 969ad691daaa8..45c9d6863ed0a 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java @@ -747,7 +747,8 @@ public FileStatus[] listStatus(final Path path, final String startFrom) throws I long contentLength = entry.contentLength() == null ? 0 : entry.contentLength(); boolean isDirectory = entry.isDirectory() == null ? false : entry.isDirectory(); if (entry.lastModified() != null && !entry.lastModified().isEmpty()) { - lastModifiedMillis = DateTimeUtils.parseLastModifiedTime(entry.lastModified()); + lastModifiedMillis = DateTimeUtils.parseLastModifiedTime( + entry.lastModified()); } Path entryPath = new Path(File.separator + entry.name()); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java index 2fa06134716a2..fa81f13344a2b 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java @@ -27,13 +27,14 @@ import org.slf4j.LoggerFactory; public final class DateTimeUtils { - public static final Logger LOG = LoggerFactory.getLogger(DateTimeUtils.class); + private static final Logger LOG = LoggerFactory.getLogger(DateTimeUtils.class); private static final String DATE_TIME_PATTERN = "E, dd MMM yyyy HH:mm:ss z"; public static long parseLastModifiedTime(final String lastModifiedTime) { long parsedTime = 0; try { - Date utcDate = new SimpleDateFormat(DATE_TIME_PATTERN, Locale.US).parse(lastModifiedTime); + Date utcDate = new SimpleDateFormat(DATE_TIME_PATTERN, Locale.US) + .parse(lastModifiedTime); parsedTime = utcDate.getTime(); } catch (ParseException e) { LOG.error("Failed to parse the date {}", lastModifiedTime); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index d51fdd84fab2c..a993060138584 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -41,13 +41,15 @@ import static java.net.HttpURLConnection.HTTP_NOT_FOUND; import static java.net.HttpURLConnection.HTTP_OK; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + 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. @@ -55,8 +57,8 @@ public class ITestAzureBlobFileSystemDelete extends AbstractAbfsIntegrationTest { - private final int reducedRetryCount = 1; - private final int reducedMaxBackoffIntervalMs = 5000; + private final static int REDUCED_RETRY_COUNT = 1; + private final static int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; public ITestAzureBlobFileSystemDelete() throws Exception { super(); @@ -152,8 +154,8 @@ 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); + abfsConfig.setMaxIoRetries(REDUCED_RETRY_COUNT); + abfsConfig.setMaxBackoffIntervalMilliseconds(REDUCED_MAX_BACKOFF_INTERVALS_MS); final AzureBlobFileSystem fs = getFileSystem(); AbfsClient abfsClient = fs.getAbfsStore().getClient(); @@ -168,14 +170,14 @@ public void testDeleteIdempotency() throws Exception { abfsConfig.getAccountName().indexOf(DOT)), abfsConfig.getStorageAccountKey()), abfsConfig, - new ExponentialRetryPolicy(reducedRetryCount), + new ExponentialRetryPolicy(REDUCED_RETRY_COUNT), abfsConfig.getTokenProvider(), tracker); // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero - when(op.getRetryCount()).thenReturn(reducedRetryCount); + when(op.getRetryCount()).thenReturn(REDUCED_RETRY_COUNT); // Mock instance of Http Operation response. This will return HTTP:Not Found AbfsHttpOperation http404Op = mock(AbfsHttpOperation.class); @@ -184,11 +186,12 @@ public void testDeleteIdempotency() throws Exception { // Mock delete response to 404 when(op.getResult()).thenReturn(http404Op); - assertTrue( + assertEquals( "Delete is considered idempotent by default and should return success.", - (testClient.deleteIdempotencyCheckOp(op) + HTTP_OK, + testClient.deleteIdempotencyCheckOp(op) .getResult() - .getStatusCode() == HTTP_OK)); + .getStatusCode()); } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 0aba3c3383c90..bb0fee1c8dde6 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -42,13 +42,15 @@ import static java.net.HttpURLConnection.HTTP_NOT_FOUND; import static java.net.HttpURLConnection.HTTP_OK; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + 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. @@ -56,8 +58,8 @@ public class ITestAzureBlobFileSystemRename extends AbstractAbfsIntegrationTest { - private final int reducedRetryCount = 1; - private final int reducedMaxBackoffIntervalMs = 5000; + private final static int REDUCED_RETRY_COUNT = 1; + private final static int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; public ITestAzureBlobFileSystemRename() throws Exception { super(); @@ -171,31 +173,41 @@ public void testPosixRenameDirectory() throws Exception { public void testRenameRetryFailureAsHTTP400() throws Exception { // Rename failed as Bad Request // RenameIdempotencyCheck should throw back the rename failure Op - testRenameTimeout(HTTP_BAD_REQUEST, HTTP_BAD_REQUEST, false); + testRenameTimeout(HTTP_BAD_REQUEST, HTTP_BAD_REQUEST, false, + "renameIdempotencyCheckOp should return rename BadRequest " + + "response itself."); } @Test public void testRenameRetryFailureAsHTTP404() throws Exception { // Rename failed as FileNotFound and the destination LMT is // within TimespanForIdentifyingRecentOperationThroughLMT - testRenameTimeout(HTTP_NOT_FOUND, HTTP_OK, false); + testRenameTimeout(HTTP_NOT_FOUND, HTTP_OK, false, + "Rename should return success response because the destination " + + "path is present and its LMT is within " + + "TimespanForIdentifyingRecentOperationThroughLMT."); } @Test public void testRenameRetryFailureWithDestOldLMT() throws Exception { // Rename failed as FileNotFound and the destination LMT is // older than TimespanForIdentifyingRecentOperationThroughLMT - testRenameTimeout(HTTP_NOT_FOUND, HTTP_NOT_FOUND, true); + testRenameTimeout(HTTP_NOT_FOUND, HTTP_NOT_FOUND, true, + "Rename should return original rename failure response " + + "because the destination path LMT is older than " + + "TimespanForIdentifyingRecentOperationThroughLMT."); } private void testRenameTimeout( int renameRequestStatus, int renameIdempotencyCheckStatus, - boolean isOldOp) throws Exception { + boolean isOldOp, + String assertMessage) throws Exception { // Config to reduce the retry and maxBackoff time for test run AbfsConfiguration abfsConfig = getConfiguration(); - abfsConfig.setMaxIoRetries(reducedRetryCount); - abfsConfig.setMaxBackoffIntervalMilliseconds(reducedMaxBackoffIntervalMs); + abfsConfig.setMaxIoRetries(REDUCED_RETRY_COUNT); + abfsConfig.setMaxBackoffIntervalMilliseconds( + REDUCED_MAX_BACKOFF_INTERVALS_MS); final AzureBlobFileSystem fs = getFileSystem(); AbfsClient abfsClient = fs.getAbfsStore().getClient(); @@ -210,7 +222,7 @@ private void testRenameTimeout( abfsConfig.getAccountName().indexOf(DOT)), abfsConfig.getStorageAccountKey()), abfsConfig, - new ExponentialRetryPolicy(reducedRetryCount), + new ExponentialRetryPolicy(REDUCED_RETRY_COUNT), abfsConfig.getTokenProvider(), tracker); @@ -218,13 +230,13 @@ private void testRenameTimeout( assertTrue( "Timestamp range for LMT update should be twice of " + "MaxIoRetries * MaxBackoffIntervalMilliseconds", - testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount) - == (2 * reducedRetryCount * reducedMaxBackoffIntervalMs)); + testClient.getTimeIntervalToTagOperationRecent(REDUCED_RETRY_COUNT) + == (2 * REDUCED_RETRY_COUNT * REDUCED_MAX_BACKOFF_INTERVALS_MS)); // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero - when(op.getRetryCount()).thenReturn(reducedRetryCount); + when(op.getRetryCount()).thenReturn(REDUCED_RETRY_COUNT); // Mock instance of Http Operation response. This will return HTTP:Bad Request AbfsHttpOperation http400Op = mock(AbfsHttpOperation.class); @@ -238,63 +250,29 @@ private void testRenameTimeout( new Path("destination" + randomUUID().toString())); if (renameRequestStatus == HTTP_BAD_REQUEST) { - // case 1: Rename failed as Bad Request - // RenameIdempotencyCheck should throw back the rename failure Op - - // Mock response - // passed in op.getResult() will be called twice. Within renameIdempotencyCheckOp, - // and as the return value from renameIdempotencyCheckOp. - // In both cases return 400 Bad Rquest response. when(op.getResult()).thenReturn(http400Op); - - assertTrue( - "renameIdempotencyCheckOp should return rename BadRequest response itself.", - (testClient.renameIdempotencyCheckOp(op, destinationPath.toUri().getPath()) - .getResult() - .getStatusCode() == renameIdempotencyCheckStatus)); - } else if (!isOldOp) { - // case 2: Rename failed as FileNotFound and the destination LMT is - // within TimespanForIdentifyingRecentOperationThroughLMT - - // Mock response - // passed in op.getResult() will be called only once. Within renameIdempotencyCheckOp, - // as return will be of GetFileStatus Op which should be 200 OK. - when(op.getResult()).thenReturn(http404Op); - - // Create the file new. - fs.create(destinationPath); - - assertTrue( - "Rename should return success response because the destination " - + "path is present and its LMT is within TimespanForIdentifyingRecentOperationThroughLMT.", - (testClient.renameIdempotencyCheckOp(op, destinationPath.toUri().getPath()) - .getResult() - .getStatusCode() == renameIdempotencyCheckStatus)); - } else { - // case 3: Rename failed as FileNotFound and the destination LMT is - // older than TimespanForIdentifyingRecentOperationThroughLMT - + } else if (renameRequestStatus == HTTP_NOT_FOUND) { // Create the file new. fs.create(destinationPath); - // sleep to cross the threshold for old LMT. - Thread.sleep( - testClient.getTimeIntervalToTagOperationRecent(reducedRetryCount)); - - // Mock response - // passed in op.getResult() will be called twice. Within renameIdempotencyCheckOp, - // as return Op. Though GetFileStatus will be called, being an older LMT, - // rename op response will be returned. when(op.getResult()).thenReturn(http404Op); - assertTrue( - "Rename should return original rename failure response because the destination " - + "path LMT is older than TimespanForIdentifyingRecentOperationThroughLMT.", - (testClient.renameIdempotencyCheckOp(op, - destinationPath.toUri().getPath()) - .getResult() - .getStatusCode() == renameIdempotencyCheckStatus)); + if (isOldOp) { + // sleep to cross the threshold for old LMT. + Thread.sleep( + testClient.getTimeIntervalToTagOperationRecent( + REDUCED_RETRY_COUNT)); + } + } + assertEquals( + assertMessage, + renameIdempotencyCheckStatus, + testClient.renameIdempotencyCheckOp(op, + destinationPath.toUri().getPath()) + .getResult() + .getStatusCode()); + } } From 4c9cdbcaa9e7c9530ce986523776d52ec1484684 Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Thu, 14 May 2020 13:51:57 +0530 Subject: [PATCH 06/11] checkstyle fix --- .../hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java | 4 ++-- .../hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index a993060138584..9392856b93834 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -57,8 +57,8 @@ public class ITestAzureBlobFileSystemDelete extends AbstractAbfsIntegrationTest { - private final static int REDUCED_RETRY_COUNT = 1; - private final static int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; + private static final int REDUCED_RETRY_COUNT = 1; + private static final int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; public ITestAzureBlobFileSystemDelete() throws Exception { super(); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index bb0fee1c8dde6..8a09a6cd30159 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -58,8 +58,8 @@ public class ITestAzureBlobFileSystemRename extends AbstractAbfsIntegrationTest { - private final static int REDUCED_RETRY_COUNT = 1; - private final static int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; + private static final int REDUCED_RETRY_COUNT = 1; + private static final int REDUCED_MAX_BACKOFF_INTERVALS_MS = 5000; public ITestAzureBlobFileSystemRename() throws Exception { super(); From db5c57826fd39894ad88a2ea0628ebbe84b48091 Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Thu, 14 May 2020 22:41:30 +0530 Subject: [PATCH 07/11] Change LMT recent detection logic --- .../constants/FileSystemConfigurations.java | 1 + .../fs/azurebfs/services/AbfsClient.java | 47 ++++--------------- .../azurebfs/services/AbfsRestOperation.java | 4 +- .../fs/azurebfs/utils/DateTimeUtils.java | 23 +++++++++ .../hadoop-azure/src/site/markdown/abfs.md | 4 +- .../ITestAzureBlobFileSystemDelete.java | 2 +- .../ITestAzureBlobFileSystemRename.java | 28 +++++------ 7 files changed, 52 insertions(+), 57 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java index e15663c2bfc36..c12631d96db57 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java @@ -82,6 +82,7 @@ public final class FileSystemConfigurations { public static final String DEFAULT_VALUE_UNKNOWN = "UNKNOWN"; public static final boolean DEFAULT_DELETE_CONSIDERED_IDEMPOTENT = true; + public static final int DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS = 5 * 60 * 1000; // 5 mins private FileSystemConfigurations() {} } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index e0b4156fc738c..208424400d97a 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -25,6 +25,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLEncoder; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -323,10 +324,11 @@ public AbfsRestOperation renamePath(String source, final String destination, fin HTTP_METHOD_PUT, url, requestHeaders); + Instant renameRequestStartTime = Instant.now(); op.execute(); if (op.getResult().getStatusCode() != HttpURLConnection.HTTP_OK) { - return renameIdempotencyCheckOp(op, destination); + return renameIdempotencyCheckOp(renameRequestStartTime, op, destination); } return op; @@ -340,14 +342,17 @@ public AbfsRestOperation renamePath(String source, final String destination, fin * interface, the logic here will detect the rename to have happened due to * the one initiated from this ABFS filesytem instance as it was retried. This * should be a corner case hence going ahead with LMT check. + * @param renameRequestStartTime startTime for the rename request * @param op Rename request REST operation response * @param destination rename destination path * @return REST operation response post idempotency check * @throws AzureBlobFileSystemException if GetFileStatus hits any exception */ - public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, + public AbfsRestOperation renameIdempotencyCheckOp( + final Instant renameRequestStartTime, + final AbfsRestOperation op, final String destination) throws AzureBlobFileSystemException { - if ((op.getRetryCount() > 0) + if ((op.isARetriedRequest()) && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) { // Server has returned HTTP 404, which means rename source no longer // exists. Check on destination status and if it has a recent LMT timestamp. @@ -358,8 +363,7 @@ public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, String lmt = destStatusOp.getResult().getResponseHeader( HttpHeaderConfigurations.LAST_MODIFIED); - if (isRecentlyModified(lmt, - getTimeIntervalToTagOperationRecent(op.getRetryCount()))) { + if (DateTimeUtils.isRecentlyModified(lmt, renameRequestStartTime)) { return destStatusOp; } } @@ -368,37 +372,6 @@ public AbfsRestOperation renameIdempotencyCheckOp(final AbfsRestOperation op, return op; } - /** - * For operations that return user-error on retry because earlier operation - * had already succeeded on server end, max timespan for the operation to have - * updated file/folder LMT should be within retryCount * max wait time for - * each retry. To include factors like clock skew and request network time - * factors, setting the timespan to be 2 times the max timespan the - * re-tries could incurr. - * @param retryCount number of retries done for the request - * @return timespan in milli-seconds within which operation should have - * occurred to qualify as recent operation. - */ - public int getTimeIntervalToTagOperationRecent(int retryCount) { - return 2 * retryCount * abfsConfiguration.getMaxBackoffIntervalMilliseconds(); - } - - /** - * Tries to identify if an operation was recently executed based on the LMT of - * a file or folder. LMT is checked to be in a time interval to determine if it - * is a recent operation. - * @param lastModifiedTime File/Folder LMT - * @param timeIntervalToTagOperationRecent time interval in - * @return true if the LMT is within timespan for recent operation, else false - */ - private boolean isRecentlyModified(final String lastModifiedTime, - final int timeIntervalToTagOperationRecent) { - long lmtEpochTime = DateTimeUtils.parseLastModifiedTime(lastModifiedTime); - long currentEpochTime = java.time.Instant.now().toEpochMilli(); - - return (currentEpochTime - lmtEpochTime) <= timeIntervalToTagOperationRecent; - } - public AbfsRestOperation append(final String path, final long position, final byte[] buffer, final int offset, final int length, final String cachedSasToken) throws AzureBlobFileSystemException { final List requestHeaders = createDefaultHeaders(); @@ -575,7 +548,7 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, * @return REST operation response post idempotency check */ public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) { - if ((op.getRetryCount() > 0) + if ((op.isARetriedRequest()) && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) && DEFAULT_DELETE_CONSIDERED_IDEMPOTENT) { // Server has returned HTTP 404, which means path no longer diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java index 67eb4145c4291..521da96e9603e 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java @@ -84,8 +84,8 @@ public List getRequestHeaders() { return requestHeaders; } - public int getRetryCount() { - return retryCount; + public boolean isARetriedRequest() { + return (retryCount > 0); } String getSasToken() { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java index fa81f13344a2b..0461869681252 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/DateTimeUtils.java @@ -20,12 +20,15 @@ import java.text.ParseException; import java.text.SimpleDateFormat; +import java.time.Instant; import java.util.Date; import java.util.Locale; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS; + public final class DateTimeUtils { private static final Logger LOG = LoggerFactory.getLogger(DateTimeUtils.class); private static final String DATE_TIME_PATTERN = "E, dd MMM yyyy HH:mm:ss z"; @@ -43,6 +46,26 @@ public static long parseLastModifiedTime(final String lastModifiedTime) { } } + /** + * Tries to identify if an operation was recently executed based on the LMT of + * a file or folder. LMT needs to be more recent that the original request + * start time. To include any clock skew with server, LMT within + * DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS from the request start time is going + * to be considered to qualify for recent operation. + * @param lastModifiedTime File/Folder LMT + * @param expectedLMTUpdateTime original request timestamp which should + * have updated the LMT on target + * @return true if the LMT is within timespan for recent operation, else false + */ + public static boolean isRecentlyModified(final String lastModifiedTime, + final Instant expectedLMTUpdateTime) { + long lmtEpochTime = DateTimeUtils.parseLastModifiedTime(lastModifiedTime); + long currentEpochTime = expectedLMTUpdateTime.toEpochMilli(); + + return ((lmtEpochTime > currentEpochTime) + || ((currentEpochTime - lmtEpochTime) <= DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS)); + } + private DateTimeUtils() { } } diff --git a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md index 77eb753930de5..f152e9dde8f2c 100644 --- a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md +++ b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md @@ -693,8 +693,8 @@ Config `fs.azure.enable.check.access` needs to be set true to enable ### Operation Idempotency -Requests failing due to server timeouts and network failures will be retried -exponentially. PUT/POST operations are idempotent and need no specific handling +Requests failing due to server timeouts and network failures will be retried. +PUT/POST operations are idempotent and need no specific handling except for Rename and Delete operations. Rename idempotency checks are made by ensuring the LastModifiedTime on destination diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index 9392856b93834..66c21d8aedb07 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -177,7 +177,7 @@ public void testDeleteIdempotency() throws Exception { // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero - when(op.getRetryCount()).thenReturn(REDUCED_RETRY_COUNT); + when(op.isARetriedRequest()).thenReturn(true); // Mock instance of Http Operation response. This will return HTTP:Not Found AbfsHttpOperation http404Op = mock(AbfsHttpOperation.class); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 8a09a6cd30159..43a55de2a68a3 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -18,6 +18,7 @@ package org.apache.hadoop.fs.azurebfs; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; @@ -41,7 +42,6 @@ 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.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,6 +50,7 @@ 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.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS; /** @@ -226,17 +227,10 @@ private void testRenameTimeout( abfsConfig.getTokenProvider(), tracker); - // Timespan for which LMT will be considered recent. - assertTrue( - "Timestamp range for LMT update should be twice of " - + "MaxIoRetries * MaxBackoffIntervalMilliseconds", - testClient.getTimeIntervalToTagOperationRecent(REDUCED_RETRY_COUNT) - == (2 * REDUCED_RETRY_COUNT * REDUCED_MAX_BACKOFF_INTERVALS_MS)); - // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero - when(op.getRetryCount()).thenReturn(REDUCED_RETRY_COUNT); + when(op.isARetriedRequest()).thenReturn(true); // Mock instance of Http Operation response. This will return HTTP:Bad Request AbfsHttpOperation http400Op = mock(AbfsHttpOperation.class); @@ -249,19 +243,22 @@ private void testRenameTimeout( Path destinationPath = fs.makeQualified( new Path("destination" + randomUUID().toString())); + Instant renameRequestStartTime = Instant.now(); + if (renameRequestStatus == HTTP_BAD_REQUEST) { when(op.getResult()).thenReturn(http400Op); } else if (renameRequestStatus == HTTP_NOT_FOUND) { // Create the file new. fs.create(destinationPath); - when(op.getResult()).thenReturn(http404Op); if (isOldOp) { - // sleep to cross the threshold for old LMT. - Thread.sleep( - testClient.getTimeIntervalToTagOperationRecent( - REDUCED_RETRY_COUNT)); + // instead of sleeping for DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS + // which will affect test run time + // will modify renameRequestStartTime to a future time so that + // lmt will qualify for old op + renameRequestStartTime = renameRequestStartTime.plusSeconds( + DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS); } } @@ -269,7 +266,8 @@ private void testRenameTimeout( assertEquals( assertMessage, renameIdempotencyCheckStatus, - testClient.renameIdempotencyCheckOp(op, + testClient.renameIdempotencyCheckOp(renameRequestStartTime, + op, destinationPath.toUri().getPath()) .getResult() .getStatusCode()); From 14be440a53dfcd63a6a89548ab8bdc2ca48d2ea1 Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Fri, 15 May 2020 13:04:48 +0530 Subject: [PATCH 08/11] test code refactored --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 2 +- .../fs/azurebfs/services/AbfsClient.java | 4 +- .../ITestAzureBlobFileSystemDelete.java | 44 ++++++--------- .../ITestAzureBlobFileSystemRename.java | 55 +++++++------------ ...TestAbfsConfigurationFieldsValidation.java | 7 +++ .../fs/azurebfs/services/TestAbfsClient.java | 23 ++++++++ 6 files changed, 71 insertions(+), 64 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index 5799c733e2fb5..b26bf53a08ff3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -765,7 +765,7 @@ public void setMaxIoRetries(int maxIoRetries) { } @VisibleForTesting - public void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) { + void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) { this.maxBackoffInterval = maxBackoffInterval; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 208424400d97a..f104e7b9c4d39 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -135,7 +135,7 @@ public String getFileSystem() { return filesystem; } - public AbfsPerfTracker getAbfsPerfTracker() { + protected AbfsPerfTracker getAbfsPerfTracker() { return abfsPerfTracker; } @@ -884,7 +884,7 @@ private void appendIfNotEmpty(StringBuilder sb, String regEx, } @VisibleForTesting - public URL getBaseUrl() { + URL getBaseUrl() { return baseUrl; } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index 66c21d8aedb07..2cf0268c08762 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -26,14 +26,14 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; +import org.assertj.core.api.Assertions; +import org.junit.Assume; 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.azurebfs.services.TestAbfsClient; import org.apache.hadoop.fs.FileAlreadyExistsException; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.Path; @@ -44,10 +44,10 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -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; @@ -151,29 +151,19 @@ public Void call() throws Exception { @Test public void testDeleteIdempotency() throws Exception { - org.junit.Assume.assumeTrue(DEFAULT_DELETE_CONSIDERED_IDEMPOTENT); + Assume.assumeTrue(DEFAULT_DELETE_CONSIDERED_IDEMPOTENT); // Config to reduce the retry and maxBackoff time for test run - AbfsConfiguration abfsConfig = getConfiguration(); - abfsConfig.setMaxIoRetries(REDUCED_RETRY_COUNT); - abfsConfig.setMaxBackoffIntervalMilliseconds(REDUCED_MAX_BACKOFF_INTERVALS_MS); + AbfsConfiguration abfsConfig + = TestAbfsConfigurationFieldsValidation.updateRetryConfigs( + getConfiguration(), + REDUCED_RETRY_COUNT, REDUCED_MAX_BACKOFF_INTERVALS_MS); final AzureBlobFileSystem fs = getFileSystem(); AbfsClient abfsClient = fs.getAbfsStore().getClient(); - AbfsPerfTracker tracker = new AbfsPerfTracker("test", - this.getAccountName(), + AbfsClient testClient = TestAbfsClient.createTestClientFromCurrentContext( + abfsClient, 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(REDUCED_RETRY_COUNT), - abfsConfig.getTokenProvider(), - tracker); - // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero @@ -186,12 +176,12 @@ public void testDeleteIdempotency() throws Exception { // Mock delete response to 404 when(op.getResult()).thenReturn(http404Op); - assertEquals( - "Delete is considered idempotent by default and should return success.", - HTTP_OK, - testClient.deleteIdempotencyCheckOp(op) - .getResult() - .getStatusCode()); + Assertions.assertThat(testClient.deleteIdempotencyCheckOp(op) + .getResult() + .getStatusCode()) + .describedAs( + "Delete is considered idempotent by default and should return success.") + .isEqualTo(HTTP_OK); } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 43a55de2a68a3..7e03ee5bcc297 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -26,32 +26,30 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; -import org.junit.Assert; +import org.assertj.core.api.Assertions; import org.junit.Test; +import org.junit.Assert; 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.azurebfs.services.TestAbfsClient; 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 java.util.UUID.randomUUID; + import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DOT; +import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS; 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.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_CLOCK_SKEW_WITH_SERVER_IN_MS; - /** * Test rename operation. @@ -205,28 +203,17 @@ private void testRenameTimeout( boolean isOldOp, String assertMessage) throws Exception { // Config to reduce the retry and maxBackoff time for test run - AbfsConfiguration abfsConfig = getConfiguration(); - abfsConfig.setMaxIoRetries(REDUCED_RETRY_COUNT); - abfsConfig.setMaxBackoffIntervalMilliseconds( - REDUCED_MAX_BACKOFF_INTERVALS_MS); + AbfsConfiguration abfsConfig + = TestAbfsConfigurationFieldsValidation.updateRetryConfigs( + getConfiguration(), + REDUCED_RETRY_COUNT, REDUCED_MAX_BACKOFF_INTERVALS_MS); final AzureBlobFileSystem fs = getFileSystem(); AbfsClient abfsClient = fs.getAbfsStore().getClient(); - AbfsPerfTracker tracker = new AbfsPerfTracker("test", - this.getAccountName(), + AbfsClient testClient = TestAbfsClient.createTestClientFromCurrentContext( + abfsClient, 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(REDUCED_RETRY_COUNT), - abfsConfig.getTokenProvider(), - tracker); - // Mock instance of AbfsRestOperation AbfsRestOperation op = mock(AbfsRestOperation.class); // Set retryCount to non-zero @@ -263,14 +250,14 @@ private void testRenameTimeout( } - assertEquals( - assertMessage, - renameIdempotencyCheckStatus, - testClient.renameIdempotencyCheckOp(renameRequestStartTime, - op, - destinationPath.toUri().getPath()) - .getResult() - .getStatusCode()); - + Assertions.assertThat(testClient.renameIdempotencyCheckOp( + renameRequestStartTime, + op, + destinationPath.toUri().getPath()) + .getResult() + .getStatusCode()) + .describedAs(assertMessage) + .isEqualTo(renameIdempotencyCheckStatus); } + } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java index 0f550d825e101..45deb9ebeec4d 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java @@ -182,4 +182,11 @@ public void testSSLSocketFactoryConfiguration() assertEquals(DelegatingSSLSocketFactory.SSLChannelMode.OpenSSL, localAbfsConfiguration.getPreferredSSLFactoryOption()); } + public static AbfsConfiguration updateRetryConfigs(AbfsConfiguration abfsConfig, + int retryCount, + int backoffTime) { + abfsConfig.setMaxIoRetries(retryCount); + abfsConfig.setMaxBackoffIntervalMilliseconds(backoffTime); + return abfsConfig; + } } \ No newline at end of file diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsClient.java index 724f57d76357d..ce9c0325a4a23 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsClient.java @@ -26,6 +26,7 @@ import org.junit.Test; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException; import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; @@ -35,6 +36,7 @@ import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APN_VERSION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.CLIENT_VERSION; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DOT; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.FORWARD_SLASH; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.JAVA_VENDOR; @@ -240,4 +242,25 @@ public void verifyUserAgentClusterType() throws Exception { .contains(DEFAULT_VALUE_UNKNOWN); } + public static AbfsClient createTestClientFromCurrentContext( + AbfsClient baseAbfsClientInstance, + AbfsConfiguration abfsConfig) + throws AzureBlobFileSystemException { + AbfsPerfTracker tracker = new AbfsPerfTracker("test", + abfsConfig.getAccountName(), + abfsConfig); + + // Create test AbfsClient + AbfsClient testClient = new AbfsClient( + baseAbfsClientInstance.getBaseUrl(), + new SharedKeyCredentials(abfsConfig.getAccountName().substring(0, + abfsConfig.getAccountName().indexOf(DOT)), + abfsConfig.getStorageAccountKey()), + abfsConfig, + new ExponentialRetryPolicy(abfsConfig.getMaxIoRetries()), + abfsConfig.getTokenProvider(), + tracker); + + return testClient; + } } From a6e090806bab4cd07928ed895721b32c2633b54e Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Fri, 15 May 2020 14:24:32 +0530 Subject: [PATCH 09/11] Remove redundant new line --- .../hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java index 2cf0268c08762..e2973968e22fe 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java @@ -47,7 +47,6 @@ 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; From 639205dbd2d348ca22be9606a632de37af2931ab Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Tue, 19 May 2020 23:27:27 +0530 Subject: [PATCH 10/11] Dummy checkin to Trigger Yetus --- .../java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index f104e7b9c4d39..35b3d93cb14d3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -32,6 +32,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; + import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory; import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; From 65801090edfe0b0d6fbfea4545d477b76b009d96 Mon Sep 17 00:00:00 2001 From: Sneha Vijayarajan Date: Tue, 19 May 2020 23:45:47 +0530 Subject: [PATCH 11/11] Undo earlier dummy checkin to Trigger Yetus --- .../java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 35b3d93cb14d3..f104e7b9c4d39 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -32,7 +32,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; - import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory; import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;