HADOOP-19045. S3A: Validate CreateSession Timeout Propagation #6470
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
steveloughran
changed the title
|
tested: s3 london; no new failures |
|
💔 -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. |
|
Looks like this can also cover HADOOP-19022 |
|
it's similar, and that test was being (mistakenly) skipped locally as i don't have a default endpoint/region set. Not going to try and clean up the whole suite.
|
|
tested: s3 london. @mukund-thakur @hiteshs can you two look at this too? |
|
|
||
| conf.setBoolean(HTTP_SIGNER_ENABLED, true); | ||
| conf.setClass(HTTP_SIGNER_CLASS_NAME, SlowSigner.class, HttpSigner.class); | ||
| Duration duration = Duration.ofMillis(10); |
There was a problem hiding this comment.
nit: add a comment saying this needs to be super less as compared to TIMEOUT_EXCEPTION_THRESHOLD as that value is used in test,
| */ | ||
| public static void skipIfNotS3ExpressBucket( | ||
| Configuration configuration) { | ||
| assume("Skipping test as bucket is an S3Express bucket", |
|
💔 -1 overall
This message was automatically generated. |
|
something has gone wrong with me and stuff testing with s3a://landsat-pds/ right now: 403 on all access. Thought it was my s3 select stuff but it seems to blow up for me everywhere now. anyway, doing this merge without worrying about it as it did work yesterday. maybe I've gone and broken my test setup |
|
S3 london with |
New test ITestCreateSessionTimeout to verify that the duration set in fs.s3a.connection.request.timeout is passed all the way down. This is done by adding a sleep() in a custom signer and verifying that it is interrupted and that an AWSApiCallTimeoutException is raised. Improves javadocs of fs.s3a.connection.request.timeout. Change-Id: I22ee270c536fe5d0a509b707e4cf0202cc930bb5
Change-Id: Ia78bc31a653895f17c715e441cc01d95a393245b
(which were only in for debugging) Change-Id: Ib85c4f735263b1f4de05bb218e00c60e75e56cf3
This is to validate that the timeout is happening at the correct place and there is no other operation which may be calling it earlier. Change-Id: Ie975577d4519cc6964297c65788cc9d306761b02
Fix testRequestTimeout() * doesn't skip if considered cross-region * sets a minimum duration of 0 before invocation * resets the minimum afterwards Change-Id: If28a54bea50da6af2b59215d1e75a47a99e5e3d7
Change-Id: Ibf8deee08ae556b264fafafebabe0afa4bdff736
steveloughran
force-pushed
the
s3/HADOOP-19045-createsession-timeouts
branch
from
dd09709 to
6113e9a
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
@steveloughran I am also getting 403 for landsat-pds since yesterday. I was planning to keep it in backlog and look into it later on but IAM has S3 full access and yet i see 403 Access denied. Looks like something has changed globally. |
|
Not only that, when i checked for any recent regression by running landsat-pds tests on relatively old branch (PR #6406 has outdated branch - 26 days older), it failed there too, with 403. |
|
ok, unrelated. we wil need to fix that. let's try some other public dataset |
asfgit
pushed a commit
that referenced
this pull request
Jan 30, 2024
New test ITestCreateSessionTimeout to verify that the duration set in fs.s3a.connection.request.timeout is passed all the way down. This is done by adding a sleep() in a custom signer and verifying that it is interrupted and that an AWSApiCallTimeoutException is raised. + Fix testRequestTimeout() * doesn't skip if considered cross-region * sets a minimum duration of 0 before invocation * resets the minimum afterwards Contributed by Steve Loughran
jiajunmao
pushed a commit
to jiajunmao/hadoop-MLEC
that referenced
this pull request
Feb 6, 2024
…#6470) New test ITestCreateSessionTimeout to verify that the duration set in fs.s3a.connection.request.timeout is passed all the way down. This is done by adding a sleep() in a custom signer and verifying that it is interrupted and that an AWSApiCallTimeoutException is raised. + Fix testRequestTimeout() * doesn't skip if considered cross-region * sets a minimum duration of 0 before invocation * resets the minimum afterwards Contributed by Steve Loughran
steveloughran
added a commit
that referenced
this pull request
Feb 7, 2024
This is a followup to PR: HADOOP-19045. S3A: Validate CreateSession Timeout Propagation (#6470) Remove all declarations of fs.s3a.connection.request.timeout in - hadoop-common/src/main/resources/core-default.xml - hadoop-aws/src/test/resources/core-site.xml New test in TestAwsClientConfig to verify that the value defined in fs.s3a.Constants class is used. This is brittle to someone overriding it in their test setups, but as this test is intended to verify that the option is not explicitly set, there's no workaround. Contributed by Steve Loughran
asfgit
pushed a commit
that referenced
this pull request
Feb 7, 2024
This is a followup to PR: HADOOP-19045. S3A: Validate CreateSession Timeout Propagation (#6470) Remove all declarations of fs.s3a.connection.request.timeout in - hadoop-common/src/main/resources/core-default.xml - hadoop-aws/src/test/resources/core-site.xml New test in TestAwsClientConfig to verify that the value defined in fs.s3a.Constants class is used. This is brittle to someone overriding it in their test setups, but as this test is intended to verify that the option is not explicitly set, there's no workaround. Contributed by Steve Loughran
slfan1989
pushed a commit
that referenced
this pull request
Feb 10, 2024
New test ITestCreateSessionTimeout to verify that the duration set in fs.s3a.connection.request.timeout is passed all the way down. This is done by adding a sleep() in a custom signer and verifying that it is interrupted and that an AWSApiCallTimeoutException is raised. + Fix testRequestTimeout() * doesn't skip if considered cross-region * sets a minimum duration of 0 before invocation * resets the minimum afterwards Contributed by Steve Loughran
slfan1989
pushed a commit
that referenced
this pull request
Feb 10, 2024
This is a followup to PR: HADOOP-19045. S3A: Validate CreateSession Timeout Propagation (#6470) Remove all declarations of fs.s3a.connection.request.timeout in - hadoop-common/src/main/resources/core-default.xml - hadoop-aws/src/test/resources/core-site.xml New test in TestAwsClientConfig to verify that the value defined in fs.s3a.Constants class is used. This is brittle to someone overriding it in their test setups, but as this test is intended to verify that the option is not explicitly set, there's no workaround. Contributed by Steve Loughran
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HADOOP-19045. Validate CreateSession Timeout Propagation
New test ITestCreateSessionTimeout to verify that the duration set
in fs.s3a.connection.request.timeout is passed all the way down.
This is done by adding a sleep() in a custom signer and verifying
that it is interrupted and that an AWSApiCallTimeoutException is
raised.
In v1 sdk this option was set to 0 because slow uploads would trigger the timeout.
But now it is only used for the Api Call Timeout value and there's an internal minimum
value fixup which sets it to 15s at a minimum. 60s is more resilient to problems.
Note this PR chain includes #6467 as that is needed for the AWS SDK containing the commit; that MUST be merged first into trunk
How was this patch tested?
S3 Express bucket in usw-2
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?