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 2022/06/21 19:00:48 UTC
[GitHub] [hadoop] taklwu opened a new pull request, #4483: HADOOP-18310 Add option and make 400 bad request retryable
taklwu opened a new pull request, #4483:
URL: https://github.com/apache/hadoop/pull/4483
### Description of PR
Add option and make 400 bad request retryable, added `fs.s3a.retry.failOnAwsBadRequest` and default to `true` such that it's acting the same behavior without turning it on.
### How was this patch tested?
Add a new unit test.
### For code changes:
- [X] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1164217094
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 56s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +0 :ok: | markdownlint | 0m 1s | | 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 21s | | trunk passed |
| +1 :green_heart: | compile | 0m 53s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 42s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 52s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 39s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 27s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 40s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 33s | | the patch passed |
| +1 :green_heart: | compile | 0m 41s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 41s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 24s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 20s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 11s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 16s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 45s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 103m 28s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4483 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint |
| uname | Linux 1c7232363729 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 60df0f6f3637b114bb3fc7d43c74dc2208268310 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/5/testReport/ |
| Max. process+thread count | 581 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1163267093
which s3 endpoint did you run the hadoop-aws integration tests against, and what was the full mvn command line used? thanks
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] taklwu commented on pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
taklwu commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1165228403
reading from the [hadoop-aws page](https://hadoop.apache.org/docs/stable/hadoop-aws/tools/hadoop-aws/index.html)
> The status code 400, Bad Request usually means that the request is unrecoverable; it’s the generic “No” response. Very rarely it does recover, which is why it is in this category, rather than that of unrecoverable failures.
That does not match the code that in fact we're not retrying for 400 bad request, in the case I'm reporting and yeah sorry it's not as usual as others, retry did help and help us to use the beauty of retry policy API
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] taklwu commented on a diff in pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
taklwu commented on code in PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#discussion_r904491406
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1177,4 +1177,8 @@ private Constants() {
*/
public static final String FS_S3A_CREATE_HEADER = "fs.s3a.create.header";
+ public static final String FAIL_ON_AWS_BAD_REQUEST = "fs.s3a.retry.failOnAwsBadRequest";
Review Comment:
ack and thanks, I will update it soon.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1163984809
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 59s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 10s | | trunk passed |
| +1 :green_heart: | compile | 0m 52s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 42s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 52s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 38s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 28s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 33s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 35s | | the patch passed |
| +1 :green_heart: | compile | 0m 41s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 41s | | the patch passed |
| +1 :green_heart: | compile | 0m 31s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 31s | | the patch passed |
| -1 :x: | blanks | 0m 0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/4/artifact/out/blanks-eol.txt) | The patch has 2 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | checkstyle | 0m 23s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 39s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 19s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 12s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 45s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 103m 10s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4483 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint |
| uname | Linux 1e6c7306374e 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f57025b0b24962cb67bfee97eb7b1c08d95e8599 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/4/testReport/ |
| Max. process+thread count | 535 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mukund-thakur commented on pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1164984485
Do we really need to introduce this config? Seems like an overkill.
I think 400 Bad Request are supposed to be non retry-able.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1162498247
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 55s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 70m 5s | | trunk passed |
| +1 :green_heart: | compile | 0m 53s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 43s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 51s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 39s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 26s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 36s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 47s | | the patch passed |
| +1 :green_heart: | compile | 0m 41s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 41s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 26s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 20s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 12s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 39s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 40s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | | The patch does not generate ASF License warnings. |
| | | 133m 43s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4483 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux edee9c8b9866 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f9292dffe2c3cdc8d351a9f87010fea36c003074 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/2/testReport/ |
| Max. process+thread count | 601 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] taklwu commented on a diff in pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
taklwu commented on code in PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#discussion_r904491346
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java:
##########
@@ -214,7 +214,10 @@ protected Map<Class<? extends Exception>, RetryPolicy> createExceptionMap() {
// policy on a 400/bad request still ambiguous.
// Treated as an immediate failure
- policyMap.put(AWSBadRequestException.class, fail);
+ RetryPolicy awsBadRequestExceptionRetryPolicy =
Review Comment:
correct me if I'm wrong but before our change, the response as `AWSBadRequestException` in fact is getting back with a HTTP 400 error code. It is different from other network failures that the `fail`/`RetryPolicies.TRY_ONCE_THEN_FAIL` has been applied for.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#discussion_r905945310
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java:
##########
@@ -214,7 +214,10 @@ protected Map<Class<? extends Exception>, RetryPolicy> createExceptionMap() {
// policy on a 400/bad request still ambiguous.
// Treated as an immediate failure
- policyMap.put(AWSBadRequestException.class, fail);
+ RetryPolicy awsBadRequestExceptionRetryPolicy =
+ configuration.getBoolean(FAIL_ON_AWS_BAD_REQUEST, DEFAULT_FAIL_ON_AWS_BAD_REQUEST) ?
+ fail : retryIdempotentCalls;
Review Comment:
1. should retry on all calls, rather than just idempotent ones, as long as we are confident that the request is never executed before the failure
2. I don't believe the normal exponential backoff strategy is the right one, as the initial delays are very short lived (500ms), whereas if you are hoping that credential providers will fetch new credentials, an initial delay of a few seconds would seem better. I wouldn't even bother with exponential growth here, just say 6 times at 10 seconds.
I think we would also want to log at warn that this is happening, assuming this is rare.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1203,4 +1203,18 @@ private Constants() {
* Default maximum read size in bytes during vectored reads : {@value}.
*/
public static final int DEFAULT_AWS_S3_VECTOR_READS_MAX_MERGED_READ_SIZE = 1253376; //1M
+
+ /**
+ * Flag for immediate failure when observing a {@link AWSBadRequestException}.
+ * If it's disabled and set to false, the failure is treated as retryable.
+ * Value {@value}.
+ */
+ public static final String FAIL_ON_AWS_BAD_REQUEST = "fs.s3a.fail.on.aws.bad.request";
Review Comment:
I now think "fs.s3a.retry.on.400.response.enabled" would be better, with default flipped. docs would say "experimental"
and assuming we do have a custom policy, adjacent
```
fs.s3a.retry.on.400.response.delay // delay between attempts, default "10s"
fs.s3a.retry.on.400.response.attempts // number of attempts, default 6
```
fs.s3a.retry.on.400
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestInvoker.java:
##########
@@ -311,12 +311,25 @@ public void testRetryAWSConnectivity() throws Throwable {
*/
@Test(expected = AWSBadRequestException.class)
public void testRetryBadRequestNotIdempotent() throws Throwable {
- invoker.retry("test", null, false,
+
+ invoker.retry("test", null, true,
() -> {
throw BAD_REQUEST;
});
}
+ @Test
+ public void testRetryBadRequestIdempotent() throws Throwable {
Review Comment:
test looks ok.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1162333506
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 55s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 11s | | trunk passed |
| +1 :green_heart: | compile | 0m 53s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 43s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 52s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 38s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 28s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 51s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 40s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 40s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 24s | [/results-checkstyle-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/1/artifact/out/results-checkstyle-hadoop-tools_hadoop-aws.txt) | hadoop-tools/hadoop-aws: The patch generated 3 new + 2 unchanged - 0 fixed = 5 total (was 2) |
| +1 :green_heart: | mvnsite | 0m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 19s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 10s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 7s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 37s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | | The patch does not generate ASF License warnings. |
| | | 103m 15s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4483 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 9e62d98eb4d1 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / a9a636cbfa48cddb8e4c37cff9e03a17736d49eb |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/1/testReport/ |
| Max. process+thread count | 577 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4483/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#discussion_r903916520
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java:
##########
@@ -214,7 +214,10 @@ protected Map<Class<? extends Exception>, RetryPolicy> createExceptionMap() {
// policy on a 400/bad request still ambiguous.
// Treated as an immediate failure
- policyMap.put(AWSBadRequestException.class, fail);
+ RetryPolicy awsBadRequestExceptionRetryPolicy =
Review Comment:
should the normal retry policy -which is expected to handle network errors- be applied here, or something else
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1177,4 +1177,8 @@ private Constants() {
*/
public static final String FS_S3A_CREATE_HEADER = "fs.s3a.create.header";
+ public static final String FAIL_ON_AWS_BAD_REQUEST = "fs.s3a.retry.failOnAwsBadRequest";
Review Comment:
1. needs to be all lower case with "." between words
2. and javadocs with {@value)
3. and something in the documentation
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] taklwu commented on pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
taklwu commented on PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#issuecomment-1163909148
@steveloughran I should have provided the test result that executed with integration tests in the description, they're not perfect but we can discuss how we move forward.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4483: HADOOP-18310 Add option and make 400 bad request retryable
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4483:
URL: https://github.com/apache/hadoop/pull/4483#discussion_r905947970
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1203,4 +1203,18 @@ private Constants() {
* Default maximum read size in bytes during vectored reads : {@value}.
*/
public static final int DEFAULT_AWS_S3_VECTOR_READS_MAX_MERGED_READ_SIZE = 1253376; //1M
+
+ /**
+ * Flag for immediate failure when observing a {@link AWSBadRequestException}.
+ * If it's disabled and set to false, the failure is treated as retryable.
+ * Value {@value}.
+ */
+ public static final String FAIL_ON_AWS_BAD_REQUEST = "fs.s3a.fail.on.aws.bad.request";
Review Comment:
I now think "fs.s3a.retry.on.400.response.enabled" would be better, with default flipped. docs would say "experimental"
and assuming we do have a custom policy, adjacent
```
fs.s3a.retry.on.400.response.delay // delay between attempts, default "10s"
fs.s3a.retry.on.400.response.attempts // number of attempts, default 6
```
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org