cloud-fan commented on code in PR #58933:
URL: https://github.com/apache/spark/pull/58933#discussion_r4068943944


##########
core/src/main/scala/org/apache/spark/scheduler/SchedulingAlgorithm.scala:
##########
@@ -28,13 +28,9 @@ private[spark] trait SchedulingAlgorithm {
 
 private[spark] class FIFOSchedulingAlgorithm extends SchedulingAlgorithm {
   override def comparator(s1: Schedulable, s2: Schedulable): Boolean = {
-    val priority1 = s1.priority
-    val priority2 = s2.priority
-    var res = math.signum(priority1 - priority2)
+    var res = Integer.compare(s1.priority, s2.priority)

Review Comment:
   **Non-blocking (P2):** Please add an in-repo regression test that fails 
against the old subtraction for both changed comparisons: priorities spanning 
`Int.MinValue` and `Int.MaxValue`, and equal-priority schedulables whose stage 
IDs span the same bounds. `PoolSuite` currently fixes priority at 0 and uses 
only small stage IDs, so it passes with the old comparator and would not catch 
a reintroduction of this scheduler-stalling failure.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to