[GitHub] [spark] cxzl25 commented on a diff in pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options

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

cxzl25 commented on code in PR #42313:
URL: https://github.com/apache/spark/pull/42313#discussion_r1285065067


##########
core/src/main/scala/org/apache/spark/SparkConf.scala:
##########
@@ -518,16 +516,19 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Seria
     }
 
     // Validate spark.executor.extraJavaOptions
-    getOption(executorOptsKey).foreach { javaOpts =>
-      if (javaOpts.contains("-Dspark")) {
-        val msg = s"$executorOptsKey is not allowed to set Spark options (was '$javaOpts'). " +
-          "Set them directly on a SparkConf or in a properties file when using ./bin/spark-submit."
-        throw new Exception(msg)
-      }
-      if (javaOpts.contains("-Xmx")) {
-        val msg = s"$executorOptsKey is not allowed to specify max heap memory settings " +
-          s"(was '$javaOpts'). Use spark.executor.memory instead."
-        throw new Exception(msg)
+    Seq(EXECUTOR_JAVA_OPTIONS.key, "spark.executor.defaultJavaOptions").foreach { executorOptsKey =>

Review Comment:
   Constants are defined in the `SparkLauncher` class, I avoid import `SparkLauncher` in `SparkConf`.
   
   https://github.com/apache/spark/blob/cf64008fce77b38d1237874b04f5ac124b01b3a8/launcher/src/main/java/org/apache/spark/launcher/SparkLauncher.java#L71



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