You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2023/08/03 04:48:43 UTC
[GitHub] [spark] cxzl25 opened a new pull request, #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
cxzl25 opened a new pull request, #42313:
URL: https://github.com/apache/spark/pull/42313
### What changes were proposed in this pull request?
### Why are the changes needed?
Command
```bash
./bin/spark-shell --conf spark.executor.extraJavaOptions='-Dspark.foo=bar'
```
Error
```
spark.executor.extraJavaOptions is not allowed to set Spark options (was '-Dspark.foo=bar'). Set them directly on a SparkConf or in a properties file when using ./bin/spark-submit.
```
Command
```bash
./bin/spark-shell --conf spark.executor.defaultJavaOptions='-Dspark.foo=bar'
```
Start up normally.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
local test
```
./bin/spark-shell --conf spark.executor.defaultJavaOptions='-Dspark.foo=bar'
```
```
spark.executor.defaultJavaOptions is not allowed to set Spark options (was '-Dspark.foo=bar'). Set them directly on a SparkConf or in a properties file when using ./bin/spark-submit.
```
--
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
[GitHub] [spark] srowen commented on pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #42313:
URL: https://github.com/apache/spark/pull/42313#issuecomment-1666859523
Merged to master
--
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
[GitHub] [spark] cxzl25 commented on a diff in pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "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
[GitHub] [spark] srowen closed pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen closed pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
URL: https://github.com/apache/spark/pull/42313
--
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
[GitHub] [spark] LuciferYang commented on pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #42313:
URL: https://github.com/apache/spark/pull/42313#issuecomment-1665110859
also cc @mridulm FYI
--
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
[GitHub] [spark] cxzl25 commented on pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #42313:
URL: https://github.com/apache/spark/pull/42313#issuecomment-1665099312
@LuciferYang @HyukjinKwon @srowen
Please help to review, thanks.
--
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
[GitHub] [spark] srowen commented on a diff in pull request #42313: [SPARK-44650][CORE] `spark.executor.defaultJavaOptions` Check illegal java options
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on code in PR #42313:
URL: https://github.com/apache/spark/pull/42313#discussion_r1284387913
##########
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:
Is there not a constant already for `spark.executor.defaultJavaOptions`? not a big deal
--
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