srowen commented on code in PR #50107:
URL: https://github.com/apache/spark/pull/50107#discussion_r1975478508
##########
core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala:
##########
@@ -169,6 +171,7 @@ private[spark] class TaskSchedulerImpl(
protected val executorIdToHost = new HashMap[String, String]
private val abortTimer =
ThreadUtils.newDaemonSingleThreadScheduledExecutor("task-abort-timer")
+ private val abortFutures = new CopyOnWriteArrayList[ScheduledFuture[_]]
Review Comment:
Why CopyOnWriteArrayList in particular? maybe fine but would a simple
synchronized list be easier?
##########
core/src/main/scala/org/apache/spark/BarrierTaskContext.scala:
##########
@@ -300,11 +300,7 @@ object BarrierTaskContext {
@Since("2.4.0")
def get(): BarrierTaskContext =
TaskContext.get().asInstanceOf[BarrierTaskContext]
- private val timer = {
- val executor = ThreadUtils.newDaemonSingleThreadScheduledExecutor(
- "Barrier task timer for barrier() calls.")
- assert(executor.isInstanceOf[ScheduledThreadPoolExecutor])
- executor.asInstanceOf[ScheduledThreadPoolExecutor]
- }
+ private val timer = ThreadUtils.newSingleThreadScheduledExecutor(
Review Comment:
This timer is never shut down. Is there a decent place to do it? otherwise,
I think daemon is correct here. Non-daemon threads would stop the JVM from
exiting, in theory
--
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
