You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Ngone51 <gi...@git.apache.org> on 2018/04/06 10:40:57 UTC
[GitHub] spark pull request #20996: [SPARK-23884][CORE] hasLaunchedTask should be tru...
GitHub user Ngone51 opened a pull request:
https://github.com/apache/spark/pull/20996
[SPARK-23884][CORE] hasLaunchedTask should be true when launchedAnyTask be true
## What changes were proposed in this pull request?
`hasLaunchedTask` should be `true` when `launchedAnyTask` be `true`, rather than `task's size > 0`.
`task'size` would be greater than 0 as long as there‘s any `WorkOffers`，but this dose not ensure there's any tasks launched.
## How was this patch tested?
exists
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Ngone51/spark SPARK-23884
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20996.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 #20996
----
commit 983926a267eb1903fd6649c31850fba9ca85d721
Author: wuyi <ng...@...>
Date: 2018-04-06T10:34:27Z
hasLaunchedTask should be true when launchedAnyTask be true
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20996: [SPARK-23884][CORE] hasLaunchedTask should be tru...
Posted by Ngone51 <gi...@git.apache.org>.
Github user Ngone51 closed the pull request at:
https://github.com/apache/spark/pull/20996
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20996: [SPARK-23884][CORE] hasLaunchedTask should be true when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20996
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 #20996: [SPARK-23884][CORE] hasLaunchedTask should be tru...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20996#discussion_r180341726
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -370,12 +370,10 @@ private[spark] class TaskSchedulerImpl(
}
if (!launchedAnyTask) {
taskSet.abortIfCompletelyBlacklisted(hostToExecutors)
+ } else {
+ hasLaunchedTask = true
--- End diff --
I don't think this is a valid change to make, IIUC `hasLaunchedTask` is to identify whether there were any resources offered, it should not have anything to do with the locality mechanism.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20996: [SPARK-23884][CORE] hasLaunchedTask should be tru...
Posted by Ngone51 <gi...@git.apache.org>.
Github user Ngone51 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20996#discussion_r180347526
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -370,12 +370,10 @@ private[spark] class TaskSchedulerImpl(
}
if (!launchedAnyTask) {
taskSet.abortIfCompletelyBlacklisted(hostToExecutors)
+ } else {
+ hasLaunchedTask = true
--- End diff --
Hi, @jiangxb1987 Thanks for your comment.
From the usage of `hasLaunchedTask ` seems it is related to resources offer. But for the name itself, it is not correct. uh, maybe, trivial.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20996: [SPARK-23884][CORE] hasLaunchedTask should be true when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20996
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 #20996: [SPARK-23884][CORE] hasLaunchedTask should be true when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20996
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