You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by coolfrood <gi...@git.apache.org> on 2014/11/19 18:35:15 UTC
[GitHub] spark pull request: [SPARK-4478] Keep totalRegisteredExecutors up-...
GitHub user coolfrood opened a pull request:
https://github.com/apache/spark/pull/3368
[SPARK-4478] Keep totalRegisteredExecutors up-to-date
totalRegisteredExecutors keeps track of the number of executors
currently registered with the scheduler. This commit fixes SPARK-4478
bug by reducing the count when an executor is lost.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/coolfrood/spark topic/SPARK-4478
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3368.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 #3368
----
commit 091919752418154fb40d3b60e119d92a92a37893
Author: Akshat Aranya <aa...@quantcast.com>
Date: 2014-11-19T17:31:37Z
[SPARK-4478] Keep totalRegisteredExecutors up-to-date
totalRegisteredExecutors keeps track of the number of executors
currently registered with the scheduler. This commit fixes SPARK-4478
bug by reducing the count when an executor is lost.
----
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63727389
[Test build #23636 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23636/consoleFull) for PR 3368 at commit [`8a4d1e4`](https://github.com/apache/spark/commit/8a4d1e4e53aa94dcb50cd6b36a64697a2e5a48b8).
* 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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63713138
@andrewor14: I agree. This field isn't actually used for anything right now; I grepped through the code. My guess would be that the original author intended it to be the current executor count instead of a total number of executors since start. I will add a comment.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63729410
Closing this PR, so that I can create a new one after the rebase.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63739404
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23636/
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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63735618
That's because you needed to pull the latest changes from master into your branch first, and then push it. Not a big deal, but it would be better if we keep the same PR open.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood closed the pull request at:
https://github.com/apache/spark/pull/3368
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63680211
Can one of the admins verify this patch?
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63724114
[Test build #23624 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23624/consoleFull) for PR 3368 at commit [`0919197`](https://github.com/apache/spark/commit/091919752418154fb40d3b60e119d92a92a37893).
* This patch **fails MiMa 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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63716028
[Test build #23630 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23630/consoleFull) for PR 3368 at commit [`456cab8`](https://github.com/apache/spark/commit/456cab885c9a94d2730343c51ab0dc6c80b16d0e).
* 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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63728918
[Test build #23630 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23630/consoleFull) for PR 3368 at commit [`456cab8`](https://github.com/apache/spark/commit/456cab885c9a94d2730343c51ab0dc6c80b16d0e).
* This patch **fails MiMa 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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63708850
Hey @coolfrood that depends on what we want `totalRegisteredExecutors` to mean. If it means "the total count of all executors that have been registered so far", then the current code is correct. If it means "the total count of executors that are currently registered", then your patch is correct. Since this is only used on start-up I think it's OK to change the meaning of it. I would add a comment above `totalRegisteredExecutors` to clarify that it means the latter, however. LGTM otherwise.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63735895
For others following, this is reopened at #3373
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63709517
[Test build #23624 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23624/consoleFull) for PR 3368 at commit [`0919197`](https://github.com/apache/spark/commit/091919752418154fb40d3b60e119d92a92a37893).
* 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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63735202
@andrewor14: I did that initially, but the Jenkins build still failed for the same problem.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63734550
You can just push to the same branch after rebasing without opening a new PR
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63739394
[Test build #23636 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23636/consoleFull) for PR 3368 at commit [`8a4d1e4`](https://github.com/apache/spark/commit/8a4d1e4e53aa94dcb50cd6b36a64697a2e5a48b8).
* 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-4478] Keep totalRegisteredExecutors up-...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63728932
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23630/
Test FAILed.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63724470
Hey @coolfrood can you rebase on master? The MiMa issue is now fixed there
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63736110
Ah, OK. I'll keep that in mind for future PRs.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63724123
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23624/
Test FAILed.
---
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-4478] Keep totalRegisteredExecutors up-...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63708335
ok 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-4478] Keep totalRegisteredExecutors up-...
Posted by coolfrood <gi...@git.apache.org>.
Github user coolfrood commented on the pull request:
https://github.com/apache/spark/pull/3368#issuecomment-63726857
@andrewor14: Done, I think. I'm not very sure about rebasing.
---
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