-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hadoop 16857. ABFS: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy #1923
Conversation
Test results: HNS not enabled account: |
🎊 +1 overall
This message was automatically generated. |
Please fix the checkstyle issues |
@snvijaya although Yetus gave +1, but new checkstyle issues are reported in the results: |
@DadanielZ - Have fixed checkstyle issues. Have retained the test input values in TestExponentialRetry class. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
...oop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java
Show resolved
Hide resolved
...oop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java
Outdated
Show resolved
Hide resolved
Latest test results: East US 2 Account - With HNS [INFO] Tests run: 54, Failures: 0, Errors: 0, Skipped: 0 East US 2 Account - Without HNS |
🎊 +1 overall
This message was automatically generated. |
Looks good to me, +1. |
hey, can people remember to update the JIRA when something is merged. Easily forgotten, but it's critical. cherrypicked to branch-3.3 and closed the JIRA |
urrent behaviour:
AzureADAuthenticator includes retry logic for all oAuth token providers that are implemented in ABFS Driver. Where as CustomTokenProvider failures end up in retry from the ABFS Driver REST request invoking layer which is not expected.
Expected behaviour:
Ideally custom token provider implementation should include retry logic too. Raising PR to stop retries getting triggered from the REST op layer due to exceptions from custom token provider.
As custom token providers might have had benefited by the unintentional retry happening, have retained a linear retry to trigger getAccessToken. The retry count is made configurable.
Testing:
The fix is tested with mock failures of a CustomTokenProvider by modifying a existing testcase that injects this scenario.
The modified testcase previously used to test a new config introduced to control the number of request retries done by exponential retry policy. New test class TestExponentialRetryPolicy is added to ensure that tests for this older config is present.