[GitHub] spark pull request #17113: [SPARK-13669][Core] Improve the blacklist mechani...

[squito <gi...@git.apache.org>]

Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17113#discussion_r106245985
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
    @@ -753,6 +753,12 @@ private[spark] class TaskSetManager(
               tasksSuccessful += 1
             }
             isZombie = true
    +
    +        if (fetchFailed.bmAddress != null) {
    +          blacklistTracker.foreach(_.updateBlacklistForFetchFailure(fetchFailed.bmAddress.host,
    +            fetchFailed.bmAddress.executorId, numTasks - tasksSuccessful))
    --- End diff --
    
    I don't think we should use `numTasks - tasksSuccessful` here.  The intent of `SparkListenerExecutorBlacklisted.taskFailures` field was meant to indicate how many failures occurred on the given executor.  In this case, its always just 1.
    
    Maybe we should really add another field "reason", or something like that, which includes whether it was for task failures or fetch failures.  Though I really don't think that is necessary ... its probably too much detail for the UI, and there is enough info in the logs for someone that needs to dive deeper.


---
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