-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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-18757: Bump corePoolSize of HadoopThreadPoolExecutor in s3a committer #5706
Conversation
@steveloughran Could you have a look into this |
💔 -1 overall
This message was automatically generated. |
which s3 endpoint did you test against, and what parameters did you use on the build. For anything related to the committers, -Dscale will include the terasorts, so I'd like that. |
Tested in |
interesting failures...it's where having everyone test in their own config helps find many config-releated issues ITestS3AFileSystemStatistic
really bizarre that one, as we are doing two full reads of 1 KB file. ITestS3ATemporaryCredentialslooks like it needs to be something which can be disabled, maybe by setting the endpoint to something special like "none". why not create a new JIRA for that for you to work on later... ITestStagingCommitProtocolFailurethat shouldn't happen; as it looks like a setup expecting a failure is now passing. Probably needs fixing in the test, but we need to understand why the test is failing first. this is the one which needs attention.
|
Sorry i couldn't get back to this earlier ITestS3AFileSystemStatisticThe test succeeds when its run by itself. I suppose it could be becuase the
ITestStagingCommitProtocolFailureAlso works when running on its own. I suppose this could also be due to some shared objects.
|
ITestS3AFileSystemStatistic
seems likely. can you create a new hadoop JIRA under HADOOP-18477 which can be used to track this. ITestStagingCommitProtocolFailure
how about this one, as it is committer related, you fix by adding
at the end of createConfiguration() |
Fixed https://issues.apache.org/jira/browse/HADOOP-18784 - ITestS3AFileSystemStatistic |
🎊 +1 overall
This message was automatically generated. |
@steveloughran Could you take a second look? As far as I can tell this makes both 3.3.5 and 3.3.6 unusable with s3 without providing an alternative committer code. |
will do; i was offline for a few days |
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.
code good, just imports to tune.
+1 pending this change
...a/org/apache/hadoop/fs/s3a/commit/staging/integration/ITestStagingCommitProtocolFailure.java
Outdated
Show resolved
Hide resolved
This reverts commit e2a25b2.
🎊 +1 overall
This message was automatically generated. |
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
…read (apache#5706) Contributed by Moditha Hewasinghage <!-- Thanks for sending a pull request! 1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute 2. Make sure your PR title starts with JIRA issue id, e.g., 'HADOOP-17799. Your PR title ...'. --> ### Description of PR ### How was this patch tested? ### For code changes: - [ ] 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?
…read (#5706) Contributed by Moditha Hewasinghage
merged to trunk; cherrypicked to branch-3.3 and ran the scale tests against s3 london; all good. thanks! |
@@ -236,7 +236,7 @@ private ExecutorService buildThreadPool( | |||
.setDaemon(true) | |||
.setNameFormat(THREAD_PREFIX + jobId + "-%d") | |||
.build(); | |||
return new HadoopThreadPoolExecutor(0, numThreads, | |||
return new HadoopThreadPoolExecutor(numThreads, numThreads, |
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.
Hi, @steveloughran . In this case, it seems that there is no workaround in Apache Hadoop 3.3.5 and 3.3.6. Do you have any recommendation for this issue? Currently, Apache Spark 3.5.0 RC1 tag is using Apache Hadoop 3.3.6.
[SPARK-44197][BUILD] Upgrade Hadoop to 3.3.6
[SPARK-42913][BUILD] Upgrade Hadoop to 3.3.5
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.
you are going to have to go with it for now; i think we should be thinking about a 3.3.7 before long as we have some other abfs and s3a issues causing pain...
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.
Thank you, @steveloughran .
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.
- a cve rollup on protobuf, guava etc... not jackson, sadly
### What changes were proposed in this pull request? This PR aims to downgrade the Apache Hadoop dependency to 3.3.4 in `Apache Spark 3.5` in order to prevent any regression from `Apache Spark 3.4.x`. In other words, although `Apache Spark 3.5.x` will lose many bug fixes of Apache Hadoop 3.3.5 and 3.3.6, it will be in the same situation with `Apache Spark 3.4.x`. - SPARK-44197 Upgrade Hadoop to 3.3.6 (#41744) - SPARK-42913 Upgrade Hadoop to 3.3.5 (#39124) - SPARK-43448 Remove dummy dependency `hadoop-openstack` (#41133) On top of reverting SPARK-44197 and SPARK-42913, this PR has additional dependency exclusion change due to the following. - SPARK-43880 Organize `hadoop-cloud` in standard maven project structure (#41380) ### Why are the changes needed? There is a community report on S3A committer performance regression. Although it's one liner fix, there is no available Hadoop release with that fix at this time. - HADOOP-18757: Bump corePoolSize of HadoopThreadPoolExecutor in s3a committer (apache/hadoop#5706) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. Closes #42345 from dongjoon-hyun/SPARK-44678. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…read (apache#5706) Contributed by Moditha Hewasinghage
…its in a single thread (apache#5706) Contributed by Moditha Hewasinghage
…its in a single thread (apache#5706) (#52) Contributed by Moditha Hewasinghage Co-authored-by: Moditha Hewasinghage <[email protected]>
Description of PR
The ThreadPoolExecutor doesn't create more threads than coreThreads if an unbounded queue is used. This leads to no the driver only committing with a single thread
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?