-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-1507][YARN]specify num of cores for AM #3806
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
Conversation
|
Can one of the admins verify this patch? |
|
SPARK_MASTER_CORES uses "master" incorrectly. The only reason we have a SPARK_MASTER_MEMORY was to preserve backwards capability. This patch also still appears to use "--driver-cores" and SPARK_DRIVER_CORES to set the AM memory in yarn-client mode, which we talked about avoiding on the previous PR. |
|
@sryza, I am not agree with you. I only add the below code into cluster mode. So the "--driver-cores" will not work in client mode. |
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.
no need to add in a deprecated "--master-cores" property
|
@XuTingjun ah, that's correct. Looking more closely, my confusion was stemming from some existing weirdness, which is that setting the "spark.driver.memory" property will set the application master memory even in client mode. Also, that passing "--driver-memory" as part of the YARN code's ClientArguments (but not to My opinion is that we should fix those, though I suppose it could be argued that it breaks backwards compatibility? |
|
@sryza, do you mean "spark.driver.memory" works in yarn-client and yarn-cluster mode, so we should use one configuration maybe named "spark.driver.cores" to set am cores in yarn-client and yarn-cluster mode? |
|
I think the best thing would be to split spark.driver.memory into spark.driver.memory and spark.yarn.am.memory, and to have the latter only work for the yarn-client AM. |
|
Yearh, I agree with you. Later I will fix this. Thanks @sryza |
|
@sryza, I have splited spark.driver.memory into spark.driver.memory and spark.yarn.am.memory. Please have a look. |
|
Sorry but I have already filed a PR that splits spark.driver.memory in #3607. Could you please check if anyone already did the same work first? |
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.
indent
|
Hi @XuTingjun there seems to be a certain degree of overlap with work in #3607. Also, my concern with this PR is that it conflates "driver" and "AM" in a few places. For instance, |
Based on top of changes in #3806. https://issues.apache.org/jira/browse/SPARK-1507 `--driver-cores` and `spark.driver.cores` for all cluster modes and `spark.yarn.am.cores` for yarn client mode. Author: WangTaoTheTonic <[email protected]> Author: WangTao <[email protected]> Closes #4018 from WangTaoTheTonic/SPARK-1507 and squashes the following commits: 01419d3 [WangTaoTheTonic] amend the args name b255795 [WangTaoTheTonic] indet thing d86557c [WangTaoTheTonic] some comments amend 43c9392 [WangTao] fix compile error b39a100 [WangTao] specify # cores for ApplicationMaster
I add some configurations below.
spark.yarn.am.cores/SPARK_MASTER_CORES/SPARK_DRIVER_CORES for yarn-client mode;
spark.driver.cores for yarn-cluster mode.