You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by lianhuiwang <gi...@git.apache.org> on 2014/07/25 11:33:40 UTC
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
GitHub user lianhuiwang opened a pull request:
https://github.com/apache/spark/pull/1589
[SPARK-2687] [yarn]amClient should remove ContainerRequest
in https://issues.apache.org/jira/browse/YARN-1902, after receving allocated containers,if amClient donot remove ContainerRequest,RM will continually allocate container for spark AppMaster.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lianhuiwang/spark amclient-remove-request
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1589.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1589
----
commit cc51f706d6388adf10293d2fca4e780b8fc64692
Author: lianhuiwang <li...@gmail.com>
Date: 2014-07-25T09:28:43Z
amClient should remove ContainerRequest
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62489097
yes, the scenario that your said is one of situation.other is:
spark requests 2 containers (yarn request total = 2)
yarn allocates 2 and give it to spark (yarn request total = 0)
spark task failed so +1 to requests (yarn request total = 1)
but in my environment, there are some mistakes in last step.number of requests is incorrect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62085093
@tgravescs i have take a look at the latest version and make sure that problem still exist. because when amClient receive containers from YARN's RM, amClient need to removeContainerRequest. if amClient donot removeContainerRequest, when it has a failure container, amClient will report numExecutorsRunning+1 ResourceRequests to Yarn. Can you understand I say? i think i should submit a new PR based on the latest version code because this PR's code is out of date.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-50128259
QA tests have started for PR 1589. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17183/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-54986483
@lianhuiwang could you take a look at the latest version of the code to see if we still need this? The description on the YARN jira isn't real clear to me and I haven't had chance to look at the patch. If it does can you please more details on when you would hit it.
Note that we are only asking once for the number of containers we need upfront and then we only add in ones that are missing, (ie allocated and then perhaps died).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62553544
Ok so are you saying spark isn't properly adding 1 back in when a executor fails? Have you verified on the YARN side the number of requests it shows? I don't see how removing requests is going to help if the number of requests on the yarn side is already 0 so just want to understand the scenario.
Do you have any steps to reproduce?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-50472853
QA tests have started for PR 1589. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17354/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62720066
I think RM will allocate more than one to spark's AM when a executor fails.
Here is a scenario:
1. spark requests 3 containers (AM & RM request total = 3)
2. RM allocates one and give it to spark (AM request total = 3 but RM request total = 2)
3. spark task failed so AM will +1 to requests (AM request total = 4)
4. RM receive AM's request and allocates one to spark (RM request total =3 (4-1)), but actually is 2
5. but next RM will allocate three to spark.
and then in appmaster's log there will be the following logs:
Ignoring container %s at host %s, since we already have the required number of containers for it.
in this scenario,when AM received one allocated container, AM donot remove one request. so it causes RM allocate more containers to AM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62393880
I think I understand what you are saying, but YARN handles removing container requests once it has been allocated.
Here is a scenario:
- spark requests 2 containers (yarn request total = 2)
- yarn allocates one and give it to spark (yarn request total = 1)
- spark task failed so +1 to requests (yarn request total = 2)
If this is not the scenario please clarify.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-50478108
QA results for PR 1589:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17354/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62895644
yes,i create new PR:https://github.com/apache/spark/pull/3245 for the latest code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-50475830
@witgo @andrewor14 please take a look at it. thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang closed the pull request at:
https://github.com/apache/spark/pull/1589
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-62722993
Ah ok, I understand now. Thanks for the explanation.
Yeah if you could upmerge this to the latest that would be great. Ideal instead of just removing the first request on the list it checks to see if it fullfilled one of its host/rack level requests and removes that one first. But then again I the preferred host stuff is broken right now so kind of hard to test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-61993745
@lianhuiwang can you answer my last comment? If not can we close this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2687] [yarn]amClient should remove Cont...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1589#issuecomment-50132308
QA results for PR 1589:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17183/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---