vicennial commented on code in PR #48665:
URL: https://github.com/apache/spark/pull/48665#discussion_r1821116057


##########
core/src/main/scala/org/apache/spark/executor/Executor.scala:
##########
@@ -559,9 +561,20 @@ private[spark] class Executor(
     override def run(): Unit = {
 
       // Classloader isolation
+      var maybeTask: Option[TimerTask] = None
       val isolatedSession = taskDescription.artifacts.state match {
         case Some(jobArtifactState) =>
-          isolatedSessionCache.get(jobArtifactState.uuid, () => 
newSessionState(jobArtifactState))
+          val state = isolatedSessionCache.get(
+            jobArtifactState.uuid, () => newSessionState(jobArtifactState))
+          maybeTask = Some(new TimerTask {
+            def run(): Unit = {
+              // Resets the expire time till the task ends.
+              val v = isolatedSessionCache.getIfPresent(jobArtifactState.uuid)
+              if (v != null) isolatedSessionCache.put(jobArtifactState.uuid, v)

Review Comment:
   Why do we need the write here? Wouldn't the access in the line above be 
sufficient to reset expiration?



##########
core/src/main/scala/org/apache/spark/executor/Executor.scala:
##########
@@ -559,9 +561,20 @@ private[spark] class Executor(
     override def run(): Unit = {
 
       // Classloader isolation
+      var maybeTask: Option[TimerTask] = None
       val isolatedSession = taskDescription.artifacts.state match {
         case Some(jobArtifactState) =>
-          isolatedSessionCache.get(jobArtifactState.uuid, () => 
newSessionState(jobArtifactState))
+          val state = isolatedSessionCache.get(
+            jobArtifactState.uuid, () => newSessionState(jobArtifactState))
+          maybeTask = Some(new TimerTask {
+            def run(): Unit = {
+              // Resets the expire time till the task ends.
+              val v = isolatedSessionCache.getIfPresent(jobArtifactState.uuid)
+              if (v != null) isolatedSessionCache.put(jobArtifactState.uuid, v)
+            }
+          })
+          maybeTask.foreach(timer.schedule(_, 60000L, 60000L))

Review Comment:
   Shall we have the time depend on the expiry of the guava cache entries? 
Perhaps `cache_expiry_time/10`



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