mridulm commented on code in PR #44321: URL: https://github.com/apache/spark/pull/44321#discussion_r1430656022
########## core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala: ########## @@ -1086,7 +1133,7 @@ private[spark] class TaskSetManager( addPendingTask(index) // Tell the DAGScheduler that this task was resubmitted so that it doesn't think our // stage finishes when a total of tasks.size tasks finish. - sched.dagScheduler.taskEnded( + emptyTaskInfoAccumulablesAndNotifyDagScheduler(tid, tasks(index), Resubmitted, null, Seq.empty, Array.empty, info) Review Comment: Yes, what @JoshRosen [detailed for executorLost](https://github.com/apache/spark/pull/44321#pullrequestreview-1785137821) is what I am referring to - had missed that comment, thanks for the analysis Josh ! Reducing memory utilization is definitely something we should aspire for, but IMO not at the cost of breaking interfaces and existing user code - in this case, `Resubmitted` has existed for 8+ years; and has been exposed to developer's. How commonly this is triggered depends on stability of the cluster infra - which might be different for different users/deployments :-) In this specific case, unfortunately it impacts not just end user code (which we dont know how it is being used: and I have seen fairly involved usage of these api's in general, including writing some of my own in the past) - but also our spark UI - for example `AppStatusListener.onTaskEnd` when `Resubmitted` is fired. If we want to relook at the behavior of how `Resubmitted` is handled in Spark, we should do that decoupled from a performance discussion - it is possible that some of this has evolved in "interesting ways" since initially designed, and requires to be revisited (why should users care, etc are discussions to be had there). For this PR, which is to improve memory utilization, it should not have unintended side effects IMO. As I mentioned before, at a minimum, this should not become default. Once we relook/redesign `Resubmitted`, we can flip it - if required. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org