[GitHub] [spark] squito commented on a change in pull request #23677: [SPARK-26755][SCHEDULER] : Optimize Spark Scheduler to dequeue speculative tasks…

[GitBox <gi...@apache.org>]

squito commented on a change in pull request #23677: [SPARK-26755][SCHEDULER] : Optimize Spark Scheduler to dequeue speculative tasks…
URL: https://github.com/apache/spark/pull/23677#discussion_r299565126
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala
 ##########
 @@ -143,36 +144,25 @@ private[spark] class TaskSetManager(
   // of failures.
   // Duplicates are handled in dequeueTaskFromList, which ensures that a
   // task hasn't already started running before launching it.
-  private val pendingTasksForExecutor = new HashMap[String, ArrayBuffer[Int]]
 
-  // Set of pending tasks for each host. Similar to pendingTasksForExecutor,
-  // but at host level.
-  private val pendingTasksForHost = new HashMap[String, ArrayBuffer[Int]]
-
-  // Set of pending tasks for each rack -- similar to the above.
-  private val pendingTasksForRack = new HashMap[String, ArrayBuffer[Int]]
-
-  // Set containing pending tasks with no locality preferences.
-  private[scheduler] var pendingTasksWithNoPrefs = new ArrayBuffer[Int]
-
-  // Set containing all pending tasks (also used as a stack, as above).
-  private val allPendingTasks = new ArrayBuffer[Int]
-
-  // Set of pending tasks that can be speculated for each executor.
-  private[scheduler] var pendingSpeculatableTasksForExecutor =
-    new HashMap[String, HashSet[Int]]
-
-  // Set of pending tasks that can be speculated for each host.
-  private[scheduler] var pendingSpeculatableTasksForHost = new HashMap[String, HashSet[Int]]
-
-  // Set of pending tasks that can be speculated with no locality preferences.
-  private[scheduler] val pendingSpeculatableTasksWithNoPrefs = new HashSet[Int]
-
-  // Set of pending tasks that can be speculated for each rack.
-  private[scheduler] var pendingSpeculatableTasksForRack = new HashMap[String, HashSet[Int]]
-
-  // Set of all pending tasks that can be speculated.
-  private[scheduler] val allPendingSpeculatableTasks = new HashSet[Int]
+  private[scheduler] val pendingTasks = PendingTasksByLocality(
+    forExecutor = new HashMap[String, ArrayBuffer[Int]],
+    forHost = new HashMap[String, ArrayBuffer[Int]],
+    noPrefs = new ArrayBuffer[Int],
+    forRack = new HashMap[String, ArrayBuffer[Int]],
+    anyPrefs = new ArrayBuffer[Int])
 
 Review comment:
   you can clean this up a bit by having all the initialization be inside the constructor (or apply method, doesn't matter)

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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