Re: [PR] [SPARK-54528][CONNECT] Close URLClassLoader eagerly to avoid OOM [spark]

["vicennial (via GitHub)" <gi...@apache.org>]

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


##########
core/src/main/scala/org/apache/spark/executor/Executor.scala:
##########
@@ -212,14 +212,41 @@ private[spark] class Executor(
   val defaultSessionState: IsolatedSessionState = newSessionState(JobArtifactState("default", None))
 
   val isolatedSessionCache: Cache[String, IsolatedSessionState] = CacheBuilder.newBuilder()
-    .maximumSize(100)
+    .maximumSize(conf.get(EXECUTOR_ISOLATED_SESSION_CACHE_SIZE))
     .expireAfterAccess(30, TimeUnit.MINUTES)
     .removalListener(new RemovalListener[String, IsolatedSessionState]() {
       override def onRemoval(
           notification: RemovalNotification[String, IsolatedSessionState]): Unit = {
         val state = notification.getValue
         // Cache is always used for isolated sessions.
         assert(!isDefaultState(state.sessionUUID))
+        // Close classloaders to release resources.
+        try {
+          state.replClassLoader match {
+            case urlClassLoader: URLClassLoader =>
+              urlClassLoader.close()
+              logInfo(log"Closed replClassLoader (URLClassLoader) for evicted session " +
+                log"${MDC(SESSION_ID, state.sessionUUID)}")
+            case _ =>
+          }
+        } catch {
+          case NonFatal(e) =>
+            logWarning(log"Failed to close replClassLoader for session " +
+              log"${MDC(SESSION_ID, state.sessionUUID)}", e)
+        }

Review Comment:
   I believe this is redundant since, for isolated sessions, the `replClassLoader` is always set to `ExecutorClassLoader` [here](https://github.com/apache/spark/blob/53ad810087fa6b1564af4092a74c87ee45c23885/core/src/main/scala/org/apache/spark/executor/Executor.scala#L1203) (and thus, never hits the match case) since the URI is always set



-- 
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-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org