You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jerryshao <gi...@git.apache.org> on 2015/03/01 06:31:19 UTC
[GitHub] spark pull request: [SPARK-5771][UI] Display the actual cores requ...
GitHub user jerryshao opened a pull request:
https://github.com/apache/spark/pull/4841
[SPARK-5771][UI] Display the actual cores requested when app is finished
Display the actual requested cores when app is finished if default core is not set, shows as below:
CC @andrewor14, is that what you intended to? My concern is that if standalone dynamic scaling is supported, it is hard to get the actual requested cores.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jerryshao/apache-spark SPAKR-5771-hotfix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4841.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 #4841
----
commit c44fd6c2cc8789fae5dadefd6f568ec3191356b5
Author: Saisai Shao <sa...@intel.com>
Date: 2015-03-01T04:56:19Z
Display the actual cores requested when app is finished
----
---
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-5771][UI] Display the actual cores requ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao closed the pull request at:
https://github.com/apache/spark/pull/4841
---
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-5771][UI] Display the actual cores requ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76584797
[Test build #28138 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28138/consoleFull) for PR 4841 at commit [`c44fd6c`](https://github.com/apache/spark/commit/c44fd6c2cc8789fae5dadefd6f568ec3191356b5).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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-5771][UI] Display the actual cores requ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76575271
[Test build #28138 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28138/consoleFull) for PR 4841 at commit [`c44fd6c`](https://github.com/apache/spark/commit/c44fd6c2cc8789fae5dadefd6f568ec3191356b5).
* This patch merges cleanly.
---
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-5771][UI] Display the actual cores requ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76627198
Hm, then `actualCoresRequested` is not a good name, since it is not the number requested. But then how is this different from `coresGranted`? there seem to be only two values here: requested, and granted. There were already two values. Why does the requested value changed after completion? that seems to be the issue. I don't see why a third value is needed.
---
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-5771][UI] Display the actual cores requ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-77287549
Hi @andrewor14 , I think your proposal is a better way to fix this issue.
Currently the name **Cores Requested** may have two meanings depending on whether app is finished or not, it is a little confused for user.
1. When app is not finished **Cores Requested** means `requestedCores` set by user.
2. When app is finished, **Cores Requested** means the actual cores requested by app.
Keep the original column **Cores** and display `coresGranted` when app is not finished, display the snapshot of `coresGranted` when app is finished is a better and consistent way.
---
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-5771][UI] Display the actual cores requ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76816413
Yes @jerryshao that is a valid concern. Dynamic allocation is one of the reasons why I'm not 100% for this feature in the first place. For now it seems OK to have this. @marsishandsome is this your original intention?
---
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-5771][UI] Display the actual cores requ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-77228784
So my proposal that we revert all of these related patches and submit the right fix, which is something like the following: Before we decrement the `coresGranted`, we check whether the application has exited. If it has, then we don't decrement it.
Note that this is not trivially easy, however, because the `Master` currently detects application completion through `DisassociatedEvent`, so there could be a race condition where the `Master` is notified of the executor's exit before the application's completion. To work around this, we need to send an explicit "unregister" message form the driver to the `Master`. I tested a rough implementation of it and verified that it works as expected.
What do others think?
---
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-5771][UI] Display the actual cores requ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76626978
`requestedCores` is the core number set by user or as a default value `Int.MaxValue`, which can be larger than actual cores gotten, `actualCoresRequested` is the actual core numbers allocated to this app.
---
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-5771][UI] Display the actual cores requ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-77227608
Let's summarize the original goals and the current state of SPARK-5771. From there we can decide how to move forward with this issue:
- Before #4567, a completed application that called `sc.stop()` will show "0 cores" on the UI, and one that did not call this will show however many cores it was using the instant before it was terminated.
- #4567 is an attempt to make this consistent, such that the number of cores displayed on the UI in both cases is non-zero. However, this shows the number of cores requested by the application, which may not make sense for applications that did not explicitly request any cores.
- #4800 hides the default number of cores (Int.MaxValue) behind `*`. This is not important to our discussion.
It seems from all of this that the deeper issue is that we use the number of cores requested by the application, which doesn't make sense for all applications. Ideally we want something that represents "the number of cores used by an application before it terminates", but the existing code in `Master` doesn't allow us to do that easily.
---
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-5771][UI] Display the actual cores requ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-77085933
Hi @srowen , maybe the name is not good. The reason why I add a third value is that:
1. The difference compared to `coresGranted` is that `coresGranted` will be changed according to the executor's adding and removing, and it will be zero when app is finished, so we cannot use this value to reflect the actual cores requested when app is finished.
2. Also as I said previously `requestedCores` is the number of cores user configured, different from the actual cores requested from application master.
Probably here `actualCoresRequested` is more likely to be `maxCoresGranted`.
---
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-5771][UI] Display the actual cores requ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76584808
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28138/
Test PASSed.
---
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-5771][UI] Display the actual cores requ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-85742914
Ok, I will go ahead and revert #4567 and #4800 to implement the solution I suggested. @jerryshao would you mind closing this PR then?
---
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-5771][UI] Display the actual cores requ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4841#issuecomment-76589806
I see the symptom here, but what is the difference between `requestedCores` and `actualCoresRequested`? why is the former changed after it completes?
---
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