bmarcott commented on a change in pull request #27207: [WIP][SPARK-18886][CORE]
Make Locality wait time measure resource under utilization due to delay
scheduling.
URL: https://github.com/apache/spark/pull/27207#discussion_r386188995
##########
File path: core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala
##########
@@ -469,8 +480,14 @@ private[spark] class TaskSetManager(
extraResources,
serializedTask)
}
+ val hasPendingTasks = pendingTasks.all.nonEmpty ||
pendingSpeculatableTasks.all.nonEmpty
Review comment:
@tgravescs
It turns out that AFAICT there is no case that would hit empty task
description, ANY locality, no pending tasks, yet there are pending speculative
tasks, because once there are no regular pending tasks, the speculative tasks
don't follow delay scheduling (you'd only get empty task if there were none
pending). The locality level is only determined by non speculative pending
tasks. See
[here](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala#L543-L546)
I ended up not adding a test because it was complex and ugly (given it isn't
currently possible).
It required overriding internal fields of TSM in odd ways.
Think it is better to add an ugly test, take out the check on speculative
tasks, or leave as is?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
