You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sadhen <gi...@git.apache.org> on 2018/04/26 10:53:05 UTC
[GitHub] spark pull request #21166: [SPARK-11334][CORE] clear idle executors in execu...
GitHub user sadhen opened a pull request:
https://github.com/apache/spark/pull/21166
[SPARK-11334][CORE] clear idle executors in executorIdToTaskIds keySet
## What changes were proposed in this pull request?
quote from #11205
> Executors may never be idle. Currently we use the executor to tasks mapping relation to identify the status of executors, in maximum task failure situation, some TaskEnd events may never be delivered, which makes the related executor always be busy.
#19580 fix the incorrect number of running tasks.
This PR solve the problem in a similar way on the `fact` that TaskEnd events may never be delivered but StageCompleted events would be delivered eventually.
## How was this patch tested?
Existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sadhen/spark SPARK-11334
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21166.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 #21166
----
commit 3f32a92352551edf79aca2c508ee575d7e3f2aa1
Author: 忍冬 <re...@...>
Date: 2018-04-26T10:46:32Z
clear idle executors in executorIdToTaskIds keySet
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/21166
1. We improve the DAGScheduler to always send TaskEnd message. So the issue I found before may not be valid.
2. We refactored the LiveListenerQueue to make it more robust for internal listener. We cannot guarantee that event will never be lost, but the chance is quite small (SPARK-18838).
IMHO you (as a PR submitter) should validate this issue with latest code and make sure you can reproduce it with latest code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21166: [SPARK-11334][CORE] clear idle executors in execu...
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen closed the pull request at:
https://github.com/apache/spark/pull/21166
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21166
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on the issue:
https://github.com/apache/spark/pull/21166
I have checked the latest master code.
``` scala
// If the executor is no longer running any scheduled tasks, mark it as idle
if (executorIdToTaskIds.contains(executorId)) {
executorIdToTaskIds(executorId) -= taskId
if (executorIdToTaskIds(executorId).isEmpty) {
executorIdToTaskIds -= executorId
allocationManager.onExecutorIdle(executorId)
}
}
```
To remove a executor, we have to mark it idle. To mark a executor idle, we have to invoke `allocationManager.onExecutorIdle`. In the latest master code, the two places that invokes the method are `onExecutorAdded` and `onTaskEnd`. For a running executor, the only way to mark it idle is the code snippet above.
#19580 makes sense on the basis that the event TaskEnd for some reason may be lost.
On the same basis, the Set `executorIdToTaskIds(executorId)` may never be empty.
> the issue is not valid any more in the latest code
Please tell me why, thanks! The TaskEnd event will never be lost or other improves ? Which PR?
We are using Spark 2.1.1, and the issue is valid.
quote from #11205 by @jerryshao
> Verified again, looks like the 2nd bullet is not valid anymore, I cannot reproduce it in latest master branch, this might have already been fixed in SPARK-13054.
Spark 2.1.1 is shipped with the fixes for SPARK-13054, but still we encountered the issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21166: [SPARK-11334][CORE] clear idle executors in execu...
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen closed the pull request at:
https://github.com/apache/spark/pull/21166
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21166
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21166: [SPARK-11334][CORE] clear idle executors in execu...
Posted by sadhen <gi...@git.apache.org>.
GitHub user sadhen reopened a pull request:
https://github.com/apache/spark/pull/21166
[SPARK-11334][CORE] clear idle executors in executorIdToTaskIds keySet
## What changes were proposed in this pull request?
quote from #11205
> Executors may never be idle. Currently we use the executor to tasks mapping relation to identify the status of executors, in maximum task failure situation, some TaskEnd events may never be delivered, which makes the related executor always be busy.
#19580 fix the incorrect number of running tasks.
This PR solves the problem in a similar way on the `fact` that TaskEnd events may never be delivered but StageCompleted events would be delivered eventually.
## How was this patch tested?
Existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sadhen/spark SPARK-11334
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21166.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 #21166
----
commit 3f32a92352551edf79aca2c508ee575d7e3f2aa1
Author: 忍冬 <re...@...>
Date: 2018-04-26T10:46:32Z
clear idle executors in executorIdToTaskIds keySet
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/21166
Can you please check again with latest master code, I doubt the issue is not valid any more in the latest code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21166: [SPARK-11334][CORE] clear idle executors in executorIdTo...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/21166
+1 seems you didn't test against the latest master code or 2.3?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org