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

    https://github.com/apache/spark/pull/11916#discussion_r57342982
  
    --- Diff: 
core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
    @@ -620,6 +620,14 @@ private[spark] class TaskSetManager(
         // Note: "result.value()" only deserializes the value when it's called 
at the first time, so
         // here "result.value()" just returns the value and won't block other 
threads.
         sched.dagScheduler.taskEnded(tasks(index), Success, result.value(), 
result.accumUpdates, info)
    +    // Kill other task attempts if any as the one attempt succeeded
    +    for (attemptInfo <- taskAttempts(index) if attemptInfo.attemptNumber 
!= info.attemptNumber
    --- End diff --
    
    Correct the SparkHadoopMapRedUtil.commitTask prevent it from actually 
committing,  but if that task completion event gets sent back to the driver 
before your code above sends kill won't it still be marked as success?  Maybe 
I'm wrong but I seem to remember seeing that happen.  If the commitTask 
happened at the right time you would see the CommitDenied, but if it happened 
later it just comes back with success and I don't see how this code helps that.
    
    I agree the second issue, there is actually another jira for it also: 
https://issues.apache.org/jira/browse/SPARK-10530, but if both are solved by 
same thing we can just dup them.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to