Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/22221#discussion_r212978188
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -465,7 +465,7 @@ private[spark] class TaskSchedulerImpl(
var reason: Option[ExecutorLossReason] = None
synchronized {
try {
- taskIdToTaskSetManager.get(tid) match {
+ Option(taskIdToTaskSetManager.get(tid)) match {
--- End diff --
It think your question is about the get() and remove() inside
cleanupTaskState all being inside a single synchronize block, correct?
I don't see that as being a problem here since taskIdToTaskSetManager is a
concurrentHashMap. That protects the operations from being atomic and if you
do a remove on an object that isn't there then it does nothing. There is no
other code that removes from there so I don't think that can happen anyway.
With this change the only thing outside of a synchronize block is a get in
accumUpdatesWithTaskIds which will be harmless if it had been removed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
