From 82658a22d6e443fcf443be01b36763cdbab96ce9 Mon Sep 17 00:00:00 2001 From: sumangala-patki <70206833+sumangala-patki@users.noreply.github.com> Date: Thu, 4 Nov 2021 20:10:37 +0530 Subject: [PATCH] HADOOP-17873. ABFS: Fix transient failures in ITestAbfsStreamStatistics and ITestAbfsRestOperationException (#3341) Addresses transient failures in the following test classes: * ITestAbfsStreamStatistics: Uses a filesystem level static instance to record read/write statistics, which also tracks these operations in other tests running in parallel. Marked for sequential-only run to avoid transient failure * ITestAbfsRestOperationException: The use of a static member to track retry count causes transient failures when two tests of this class happen to run together. Switch to non-static variable for assertions on retry count closes #3341 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 1896e15d27..075716e91e 100644 --- a/hadoop-tools/hadoop-azure/pom.xml +++ b/hadoop-tools/hadoop-azure/pom.xml @@ -555,6 +555,7 @@ **/azurebfs/ITestAzureBlobFileSystemListStatus.java **/azurebfs/extensions/ITestAbfsDelegationTokens.java **/azurebfs/ITestSmallWriteOptimization.java + **/azurebfs/ITestAbfsStreamStatistics*.java **/azurebfs/services/ITestReadBufferManager.java @@ -597,6 +598,7 @@ **/azurebfs/extensions/ITestAbfsDelegationTokens.java **/azurebfs/ITestSmallWriteOptimization.java **/azurebfs/services/ITestReadBufferManager.java + **/azurebfs/ITestAbfsStreamStatistics*.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 14914101e5..6ae0962fbf 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.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.thirdparty.com.google.common.base.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 17d5c03a81..643cb66fa2 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 @@ -1223,4 +1223,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 56d553819f..e2c274c3b3 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; @@ -241,6 +243,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 a71e7bc815..b1705cad01 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 @@ -111,7 +111,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, ()-> { @@ -119,10 +122,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(); + } }