You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/03/30 09:29:59 UTC

[GitHub] [hadoop] snvijaya opened a new pull request #1923: Hadoop 16857

snvijaya opened a new pull request #1923: Hadoop 16857
URL: https://github.com/apache/hadoop/pull/1923
 
 
   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. 
   
   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.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on issue #1923: Hadoop 16857

Posted by GitBox <gi...@apache.org>.
snvijaya commented on issue #1923: Hadoop 16857
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-605888782
 
 
   Test results:
   HNS enabled account:
   [INFO] Tests run: 54, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 412, Failures: 0, Errors: 0, Skipped: 66
   [WARNING] Tests run: 206, Failures: 0, Errors: 0, Skipped: 140
   
   HNS not enabled account:
   [INFO] Tests run: 54, Failures: 0, Errors: 0, Skipped: 0
   [WARNING] Tests run: 412, Failures: 0, Errors: 0, Skipped: 240
   [WARNING] Tests run: 206, Failures: 0, Errors: 0, Skipped: 140

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-605921218
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 29s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 31s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 25s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 12s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 54s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 52s |  trunk passed  |
   | -0 :warning: |  patch  |   1m 11s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 24s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 17s |  hadoop-tools/hadoop-azure: The patch generated 11 new + 2 unchanged - 0 fixed = 13 total (was 2)  |
   | +1 :green_heart: |  mvnsite  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m 34s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 23s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 58s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1923 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux b1134bedbdfb 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 960c9eb |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/1/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/1/testReport/ |
   | Max. process+thread count | 423 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/1/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-607346944
 
 
   Please fix the checkstyle issues

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-610190390
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  1s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  22m  1s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 31s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 25s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 33s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 10s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 51s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 48s |  trunk passed  |
   | -0 :warning: |  patch  |   1m  7s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m  5s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   0m 55s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 24s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  61m  1s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1923 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux 69907f4fc0dd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 0b855b9 |
   | Default Java | 1.8.0_242 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/5/testReport/ |
   | Max. process+thread count | 419 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/5/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya edited a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya edited a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609666674
 
 
   @DadanielZ - Addressed the checkstyle comments as mentioned above. Kindly request you to re-review. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609747634
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m  2s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 25s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m  5s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 52s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 49s |  trunk passed  |
   | -0 :warning: |  patch  |   1m  8s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 17s |  hadoop-tools/hadoop-azure: The patch generated 3 new + 2 unchanged - 0 fixed = 5 total (was 2)  |
   | +1 :green_heart: |  mvnsite  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 31s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   0m 55s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 23s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  The patch does not generate ASF License warnings.  |
   |  |   |  57m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1923 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux d9eb6757a883 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / ab7495d |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/4/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/4/testReport/ |
   | Max. process+thread count | 414 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/4/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on a change in pull request #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#discussion_r405289808
 
 

 ##########
 File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java
 ##########
 @@ -46,16 +48,53 @@
    *
    * @param adaptee the custom token provider
    */
-  public CustomTokenProviderAdapter(CustomTokenProviderAdaptee adaptee) {
+  public CustomTokenProviderAdapter(CustomTokenProviderAdaptee adaptee, int customTokenFetchRetryCount) {
     Preconditions.checkNotNull(adaptee, "adaptee");
     this.adaptee = adaptee;
+    fetchTokenRetryCount = customTokenFetchRetryCount;
   }
 
   protected AzureADToken refreshToken() throws IOException {
     LOG.debug("AADToken: refreshing custom based token");
 
     AzureADToken azureADToken = new AzureADToken();
-    azureADToken.setAccessToken(adaptee.getAccessToken());
+
+    String accessToken = null;
+
+    Exception ex;
+    boolean succeeded = false;
+    // Custom token providers should have their own retry policies,
+    // Providing a linear retry option for the the retry count
+    // mentioned in config "fs.azure.custom.token.fetch.retry.count"
+    int retryCount = fetchTokenRetryCount;
+    do {
+      ex = null;
+      try {
+        accessToken = adaptee.getAccessToken();
+        LOG.trace("CustomTokenProvider Access token fetch was successful with retry count {}", retryCount);
 
 Review comment:
   Is it necessary to add trace log here? it affects the perf for all rereshToken requests including those succeed without retry.
   And the `retryCount` in the message is confusing,  the total times of retries that have been executed should be `fetchTokenRetryCount - retryCount`, same for the msg in the `catch` block.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-608706635
 
 
   @snvijaya although Yetus gave +1, but new checkstyle issues are reported in the results:
   https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/1/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt
   please fix them

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya removed a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya removed a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609666674
 
 
   @DadanielZ - Addressed the checkstyle comments as mentioned above. Kindly request you to re-review. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609666428
 
 
   Have fixed the checkstyle issues in AbfsConfiguration and AzureADAuthenticator. Did not make changes to TestExponentialRetryPolicy.java ones as the highlighted error are for 
   1. test input numbers
   2. formatting done for method call with arguments in next line starting comma. Retained this as this was what Steve has suggested me to follow in previous PR which he mentioned provides better readability. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609719261
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  24m 53s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 32s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 24s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 19s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   | -0 :warning: |  patch  |   1m 15s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 24s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 18s |  hadoop-tools/hadoop-azure: The patch generated 3 new + 2 unchanged - 0 fixed = 5 total (was 2)  |
   | +1 :green_heart: |  mvnsite  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 27s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   0m 55s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 22s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  84m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1923 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux ccd052169bdd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / ab7495d |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/2/testReport/ |
   | Max. process+thread count | 413 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/2/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya removed a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya removed a comment on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609666428
 
 
   Have fixed the checkstyle issues in AbfsConfiguration and AzureADAuthenticator. Did not make changes to TestExponentialRetryPolicy.java ones as the highlighted error are for 
   1. test input numbers
   2. formatting done for method call with arguments in next line starting comma. Retained this as this was what Steve has suggested me to follow in previous PR which he mentioned provides better readability. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609725529
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 22s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  22m 17s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 21s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 30s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  16m 41s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 52s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 50s |  trunk passed  |
   | -0 :warning: |  patch  |   1m  8s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 14s |  hadoop-tools/hadoop-azure: The patch generated 3 new + 2 unchanged - 0 fixed = 5 total (was 2)  |
   | +1 :green_heart: |  mvnsite  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 26s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 19s |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  The patch does not generate ASF License warnings.  |
   |  |   |  64m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1923 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux b708a44eaeb9 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / ab7495d |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/3/artifact/out/diff-checkstyle-hadoop-tools_hadoop-azure.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/3/testReport/ |
   | Max. process+thread count | 368 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1923/3/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609666674
 
 
   @DadanielZ - Address the cehckstyle comments as mentioned above. Kindly request you to re-review. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] DadanielZ commented on a change in pull request #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
DadanielZ commented on a change in pull request #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#discussion_r405284480
 
 

 ##########
 File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/CustomTokenProviderAdapter.java
 ##########
 @@ -46,16 +48,53 @@
    *
    * @param adaptee the custom token provider
    */
-  public CustomTokenProviderAdapter(CustomTokenProviderAdaptee adaptee) {
+  public CustomTokenProviderAdapter(CustomTokenProviderAdaptee adaptee, int customTokenFetchRetryCount) {
     Preconditions.checkNotNull(adaptee, "adaptee");
     this.adaptee = adaptee;
+    fetchTokenRetryCount = customTokenFetchRetryCount;
   }
 
   protected AzureADToken refreshToken() throws IOException {
     LOG.debug("AADToken: refreshing custom based token");
 
     AzureADToken azureADToken = new AzureADToken();
-    azureADToken.setAccessToken(adaptee.getAccessToken());
+
+    String accessToken = null;
+
+    Exception ex;
+    boolean succeeded = false;
+    // Custom token providers should have their own retry policies,
+    // Providing a linear retry option for the the retry count
+    // mentioned in config "fs.azure.custom.token.fetch.retry.count"
+    int retryCount = fetchTokenRetryCount;
+    do {
+      ex = null;
+      try {
+        accessToken = adaptee.getAccessToken();
+        LOG.trace("CustomTokenProvider Access token fetch was successful with retry count {}", retryCount);
+      } catch (Exception e) {
 
 Review comment:
   `Throwable` might be safer?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy

Posted by GitBox <gi...@apache.org>.
snvijaya commented on issue #1923: Hadoop 16857: Stop CustomTokenProvider retry logic to depend on AbfsRestOp retry policy
URL: https://github.com/apache/hadoop/pull/1923#issuecomment-609689878
 
 
   @DadanielZ  - Have fixed checkstyle issues. Have retained the test input values in TestExponentialRetry class.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org