-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-18180. Replaces ExecutorServicePool with ExecutorService #4520
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-18180. Replaces ExecutorServicePool with ExecutorService #4520
Conversation
This is the the initial merge of the HADOOP-18028 S3A performance input stream. This patch on its own is incomplete and must be accompanied by all other commits with HADOOP-18028 in their git commit message. Consult the JIRA for that list Contributed by Bhalchandra Pandit.
…3A prefetching stream (apache#4115) Contributed by PJ Fanning.
Contributed by Ahmar Suhail
…ache#4212) Contributed by Monthon Klongklaew
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.
I have a concern on which thread pool should be passed by S3AFileSystem; otherwise, looking good.
| futurePool, | ||
| unboundedThreadPool, |
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 not the boundedThreadPool originally wrapped by futurePool?
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.
that's my bad, it should be boundedThreadPool. Have updated
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AReadOpContext.java
Show resolved
Hide resolved
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 (non-binding) assuming Yetus is happy
|
💔 -1 overall
This message was automatically generated. |
|
Failing unit tests are fixed in #4386 |
|
💔 -1 overall
This message was automatically generated. |
apache#4386) * adds in new test for prefetching input stream * creates streamStats before opening stream * updates numBlocks calculation method * fixes ITestS3AOpenCost.testOpenFileLongerLength * drains stream async * fixes failing unit test Contributed by Ahmar Suhail
Contributed by Ahmar Suhail <[email protected]>
This adds iOStatisticsConnection to the S3PrefetchingInputStream class, with new statistic names in StreamStatistics. This stream is not (yet) IOStatisticsContext aware. Contributed by Ahmar Suhail
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
a964fbf to
e23f70a
Compare
Description of PR
The
ExecutorServicePoolclass was only submitting tasks to theExecutorService. This PR removesExecutorServicePooland submits directly to theExecutorService.How was this patch tested?
Tested in eu-west-1 by running
mvn -Dparallel-tests -DtestsThreadCount=16 clean verify