-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19210. S3A: Speed up some slow unit tests #6907
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-19210. S3A: Speed up some slow unit tests #6907
Conversation
Speed up slow tests * TestS3AAWSCredentialsProvider: decrease thread pool shutdown time * TestS3AInputStreamRetry: reduce retry limit and intervals On a test run with -Dparallel-tests -DtestsThreadCount=10 this reduces test execution time from 03:00 min to 02:26 min. Change-Id: I87c731543a3868449309fe1cd445942182ffd00b
|
🎊 +1 overall
This message was automatically generated. |
|
need some reviews here. @mukund-thakur @ahmarsuhail @HarshitGupta11 . This is for your benefit as much as mine |
| .assertThat(credentials.size()) | ||
| .describedAs("List of Credentials providers") | ||
| .isEqualTo(3); | ||
| .isEqualTo(TERMINATION_TIMEOUT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is credentials.size() equal to TERMINATION_TIMEOUT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh, good catch. I pulled out the constant and must have found that as a match. will fix
Change-Id: I9c5ab2159d25b7a78d0fec64407362aec29c0374
ahmarsuhail
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM
|
|
||
| // tight retry logic as all failures are simulated | ||
| final String interval = "1ms"; | ||
| final int limit = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this change to 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can do, but it's nice and fast anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, I'd rather not. because this test is shared with other mock tests and it is probably a bit less risky to cut back on all of them just to speed up one test
|
🎊 +1 overall
This message was automatically generated. |
mukund-thakur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1
|
FYI @saikatroy038 |
Speed up slow tests * TestS3AAWSCredentialsProvider: decrease thread pool shutdown time * TestS3AInputStreamRetry: reduce retry limit and intervals Contributed by Steve Loughran
Speed up slow tests * TestS3AAWSCredentialsProvider: decrease thread pool shutdown time * TestS3AInputStreamRetry: reduce retry limit and intervals Contributed by Steve Loughran
Speed up slow tests * TestS3AAWSCredentialsProvider: decrease thread pool shutdown time * TestS3AInputStreamRetry: reduce retry limit and intervals Contributed by Steve Loughran
HADOOP-19210
Speed up slow tests
On a test run with -Dparallel-tests -DtestsThreadCount=10 this reduces test execution time from 03:00 min to 02:26 min.
How was this patch tested?
reran old tests and measured time difference.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?