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

    https://github.com/apache/spark/pull/13603#discussion_r66646791
  
    --- Diff: 
core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
    @@ -404,6 +404,25 @@ private[spark] class TaskSetManager(
       }
     
       /**
    +   * Return some task which is pending, but do not remove it from the list 
of pending tasks.
    +   * Used as a simple way to test if this task set is schedulable 
anywhere, or if it has been
    +   * completely blacklisted.
    +   */
    +  def pollPendingTask: Option[Int] = {
    +    // usually this will just take the last pending task, but because of 
the lazy removal
    +    // from each list, we may need to go deeper in the list
    +    var indexOffset = allPendingTasks.size
    +    while (indexOffset > 0) {
    +      indexOffset -= 1
    +      val index = allPendingTasks(indexOffset)
    +      if (copiesRunning(index) == 0 && !successful(index)) {
    +        return Some(index)
    +      }
    --- End diff --
    
    I'm *pretty* sure that we could add
    
    ```scala
    else {
      // this task has already been scheduled from one of our other task 
queues, so remove it 
      // from this one as well, even though we're not actually scheduling 
anything here.
      allPendingTasks.remove(indexOffset)
    }
    ```
    
    But its shouldn't be *necessary* to do here, and I'm just nervous enough 
about adding it that I opted not to.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to