From 7bcf853ff458694ef09ce8a2869ee6209e8bad4c Mon Sep 17 00:00:00 2001 From: Sumangala Patki <70206833+sumangala-patki@users.noreply.github.com> Date: Tue, 6 Sep 2022 15:30:52 +0530 Subject: [PATCH] HADOOP-17873. ABFS: Fix transient failures in ITestAbfsStreamStatistics and ITestAbfsRestOperationException (#3699) Successor for the reverted PR #3341, using the hadoop @VisibleForTesting attribute Contributed by Sumangala Patki --- hadoop-tools/hadoop-azure/pom.xml | 2 ++ .../oauth2/CustomTokenProviderAdapter.java | 6 ++++ .../fs/azurebfs/services/AbfsClient.java | 5 ++++ .../azurebfs/AbstractAbfsIntegrationTest.java | 5 ++++ .../ITestAbfsRestOperationException.java | 13 +++++---- .../oauth2/RetryTestTokenProvider.java | 29 ++++++++++++++----- .../fs/azurebfs/services/TestAbfsClient.java | 4 +++ 7 files changed, 51 insertions(+), 13 deletions(-) diff --git a/hadoop-tools/hadoop-azure/pom.xml b/hadoop-tools/hadoop-azure/pom.xml index 40aeec0702..c313fa28a3 100644 --- a/hadoop-tools/hadoop-azure/pom.xml +++ b/hadoop-tools/hadoop-azure/pom.xml @@ -604,6 +604,7 @@ **/azurebfs/ITestAzureBlobFileSystemListStatus.java **/azurebfs/extensions/ITestAbfsDelegationTokens.java **/azurebfs/ITestSmallWriteOptimization.java + **/azurebfs/ITestAbfsStreamStatistics*.java **/azurebfs/services/ITestReadBufferManager.java **/azurebfs/commit/*.java @@ -647,6 +648,7 @@ **/azurebfs/extensions/ITestAbfsDelegationTokens.java **/azurebfs/ITestSmallWriteOptimization.java **/azurebfs/services/ITestReadBufferManager.java + **/azurebfs/ITestAbfsStreamStatistics*.java **/azurebfs/commit/*.java diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java index a4ec35e2f6..889976041d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.net.URI; +import org.apache.hadoop.classification.VisibleForTesting; import org.apache.hadoop.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -138,4 +139,9 @@ public String getUserAgentSuffix() { String suffix = ExtensionHelper.getUserAgentSuffix(adaptee, ""); return suffix != null ? suffix : ""; } + + @VisibleForTesting + protected CustomTokenProviderAdaptee getCustomTokenProviderAdaptee() { + return adaptee; + } } 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 0c0660c6f6..aa72ed64e6 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 @@ -1289,4 +1289,9 @@ public ListenableFuture submit(Runnable runnable) { public void addCallback(ListenableFuture future, FutureCallback callback) { Futures.addCallback(future, callback, executorService); } + + @VisibleForTesting + protected AccessTokenProvider getTokenProvider() { + return tokenProvider; + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java index 2c99267d91..c5bf85a4f8 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java @@ -37,10 +37,12 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.azurebfs.constants.FSOperationType; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException; +import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider; import org.apache.hadoop.fs.azurebfs.security.AbfsDelegationTokenManager; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream; import org.apache.hadoop.fs.azurebfs.services.AuthType; +import org.apache.hadoop.fs.azurebfs.services.TestAbfsClient; import org.apache.hadoop.fs.azure.AzureNativeFileSystemStore; import org.apache.hadoop.fs.azure.NativeAzureFileSystem; import org.apache.hadoop.fs.azure.metrics.AzureFileSystemInstrumentation; @@ -251,6 +253,9 @@ public Hashtable call() throws Exception { } } + public AccessTokenProvider getAccessTokenProvider(final AzureBlobFileSystem fs) { + return TestAbfsClient.getAccessTokenProvider(fs.getAbfsStore().getClient()); + } public void loadConfiguredFileSystem() throws Exception { // disable auto-creation of filesystem diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsRestOperationException.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsRestOperationException.java index ce9415a817..3fe3557d50 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsRestOperationException.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsRestOperationException.java @@ -112,7 +112,10 @@ public void testWithDifferentCustomTokenFetchRetry(int numOfRetries) throws Exce final AzureBlobFileSystem fs1 = (AzureBlobFileSystem) FileSystem.newInstance(fs.getUri(), config); - RetryTestTokenProvider.ResetStatusToFirstTokenFetch(); + RetryTestTokenProvider retryTestTokenProvider + = RetryTestTokenProvider.getCurrentRetryTestProviderInstance( + getAccessTokenProvider(fs1)); + retryTestTokenProvider.resetStatusToFirstTokenFetch(); intercept(Exception.class, ()-> { @@ -120,10 +123,10 @@ public void testWithDifferentCustomTokenFetchRetry(int numOfRetries) throws Exce }); // Number of retries done should be as configured - Assert.assertTrue( - "Number of token fetch retries (" + RetryTestTokenProvider.reTryCount - + ") done, does not match with fs.azure.custom.token.fetch.retry.count configured (" + numOfRetries - + ")", RetryTestTokenProvider.reTryCount == numOfRetries); + Assert.assertEquals( + "Number of token fetch retries done does not match with fs.azure" + + ".custom.token.fetch.retry.count configured", numOfRetries, + retryTestTokenProvider.getRetryCount()); } @Test diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/oauth2/RetryTestTokenProvider.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/oauth2/RetryTestTokenProvider.java index 3566ebbaaa..7427add290 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/oauth2/RetryTestTokenProvider.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/oauth2/RetryTestTokenProvider.java @@ -30,12 +30,12 @@ */ public class RetryTestTokenProvider implements CustomTokenProviderAdaptee { - // Need to track first token fetch otherwise will get counted as a retry too. - private static boolean isThisFirstTokenFetch = true; - public static int reTryCount = 0; + private static final Logger LOG = LoggerFactory.getLogger( + RetryTestTokenProvider.class); - private static final Logger LOG = LoggerFactory - .getLogger(RetryTestTokenProvider.class); + // Need to track first token fetch otherwise will get counted as a retry too. + private boolean isThisFirstTokenFetch = true; + private int retryCount = 0; @Override public void initialize(Configuration configuration, String accountName) @@ -43,9 +43,13 @@ public void initialize(Configuration configuration, String accountName) } - public static void ResetStatusToFirstTokenFetch() { + /** + * Clear earlier retry details and reset RetryTestTokenProvider instance to + * state of first access token fetch call. + */ + public void resetStatusToFirstTokenFetch() { isThisFirstTokenFetch = true; - reTryCount = 0; + retryCount = 0; } @Override @@ -53,7 +57,7 @@ public String getAccessToken() throws IOException { if (isThisFirstTokenFetch) { isThisFirstTokenFetch = false; } else { - reTryCount++; + retryCount++; } LOG.debug("RetryTestTokenProvider: Throw an exception in fetching tokens"); @@ -64,4 +68,13 @@ public String getAccessToken() throws IOException { public Date getExpiryTime() { return new Date(); } + + public static RetryTestTokenProvider getCurrentRetryTestProviderInstance( + AccessTokenProvider customTokenProvider) { + return (RetryTestTokenProvider) ((CustomTokenProviderAdapter) customTokenProvider).getCustomTokenProviderAdaptee(); + } + + public int getRetryCount() { + return retryCount; + } } 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 a725bf3175..0a1dca7e7d 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 @@ -395,4 +395,8 @@ public static AbfsRestOperation getRestOp(AbfsRestOperationType type, url, requestHeaders); } + + public static AccessTokenProvider getAccessTokenProvider(AbfsClient client) { + return client.getTokenProvider(); + } }