You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zhichao-li <gi...@git.apache.org> on 2015/04/10 09:51:17 UTC
[GitHub] spark pull request: [SPARK-6843][core]Add volatile for the "state"
GitHub user zhichao-li opened a pull request:
https://github.com/apache/spark/pull/5448
[SPARK-6843][core]Add volatile for the "state"
Fix potential visibility problem for the "state" of Executor
The field of "state" is shared and modified by multiple threads. i.e:
```scala
Within ExecutorRunner.scala
(1) workerThread = new Thread("ExecutorRunner for " + fullId) {
override def run() { fetchAndRunExecutor() }
}
workerThread.start()
// Shutdown hook that kills actors on shutdown.
(2)shutdownHook = new Thread() {
override def run() {
killProcess(Some("Worker shutting down"))
}
}
(3)and also the "Actor thread" for worker.
```
I think we should at lease add volatile to ensure the visibility among threads otherwise the worker might send an out-of-date status to the master.
https://issues.apache.org/jira/browse/SPARK-6843
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhichao-li/spark state
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5448.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 #5448
----
commit a2386e77bcf5db2dc90e8bb63c4c52535ff7c936
Author: lisurprise <zh...@intel.com>
Date: 2015-04-10T07:36:04Z
add volatile for state field
----
---
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-6843][core]Add volatile for the "state"
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/5448#issuecomment-91532200
I think that's technically right, although I find there are probably hundreds more fields in Spark that should be `@volatile` (or else designed so as to not be shared state across threads). It's OK to fix but rather than peck at the problem, can you identify any logically similar changes that need to happen? in this class, or, in similar 'state' variables? I don't mean to evaluate every field in the code base, but just to see if we can make this about more than one particular instance.
---
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-6843][core]Add volatile for the "state"
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5448#issuecomment-91470105
[Test build #30020 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30020/consoleFull) for PR 5448 at commit [`a2386e7`](https://github.com/apache/spark/commit/a2386e77bcf5db2dc90e8bb63c4c52535ff7c936).
---
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-6843][core]Add volatile for the "state"
Posted by zhichao-li <gi...@git.apache.org>.
Github user zhichao-li commented on the pull request:
https://github.com/apache/spark/pull/5448#issuecomment-91471743
From the the code of worker , we can see it pass through the state without any guard, but the state of executors might be changed by the other threads.
```scala
case RequestWorkerState =>
sender ! WorkerStateResponse(host, port, workerId, **executors.values.toList**,
finishedExecutors.values.toList, drivers.values.toList,
finishedDrivers.values.toList, activeMasterUrl, cores, memory,
coresUsed, memoryUsed, activeMasterWebUiUrl)
```
---
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-6843][core]Add volatile for the "state"
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5448#issuecomment-91490749
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30020/
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-6843][core]Add volatile for the "state"
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5448#issuecomment-91490722
[Test build #30020 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30020/consoleFull) for PR 5448 at commit [`a2386e7`](https://github.com/apache/spark/commit/a2386e77bcf5db2dc90e8bb63c4c52535ff7c936).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
* This patch does not change any dependencies.
---
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-6843][core]Add volatile for the "state"
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5448
---
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