[GitHub] [spark] mridulm commented on a diff in pull request #39674: [SPARK-42149][YARN] Remove the env `SPARK_USE_CONC_INCR_GC` used to enable CMS GC for Yarn AM

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

mridulm commented on code in PR #39674:
URL: https://github.com/apache/spark/pull/39674#discussion_r1083387253


##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala:
##########
@@ -1005,26 +1005,6 @@ private[spark] class Client(
     val tmpDir = new Path(Environment.PWD.$$(), YarnConfiguration.DEFAULT_CONTAINER_TEMP_DIR)
     javaOpts += "-Djava.io.tmpdir=" + tmpDir
 
-    // TODO: Remove once cpuset version is pushed out.
-    // The context is, default gc for server class machines ends up using all cores to do gc -
-    // hence if there are multiple containers in same node, Spark GC affects all other containers'
-    // performance (which can be that of other Spark containers)
-    // Instead of using this, rely on cpusets by YARN to enforce "proper" Spark behavior in
-    // multi-tenant environments. Not sure how default Java GC behaves if it is limited to subset
-    // of cores on a node.
-    val useConcurrentAndIncrementalGC = launchEnv.get("SPARK_USE_CONC_INCR_GC").exists(_.toBoolean)

Review Comment:
   Agree, this is pretty old ... I dont think there are a lot of usages of this now.
   Should be fine to remove - and if there are concerns, users can explicitly add it anyway - like Tom suggested.



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