[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20645 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20645 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/975/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20645 **[Test build #87569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87569/testReport)** for PR 20645 at commit [`89ee1f3`](https://github.com/apache/spark/commit/89ee1f330047c6c8a986e62e8201d9b7890cf4c8). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20645 **[Test build #87569 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87569/testReport)** for PR 20645 at commit [`89ee1f3`](https://github.com/apache/spark/commit/89ee1f330047c6c8a986e62e8201d9b7890cf4c8). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20645 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20645 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87569/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user rdblue commented on the issue: https://github.com/apache/spark/pull/20645 @vanzin, you might be interested in this one because it makes applying administrator settings for Spark much easier. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/20645 I like the functionality, not necessarily the implementation. Other configs would benefit from this, and having this functionality more easily available without having to change the call sites would make it more useful. I'm also not a big fan of "default" being used in the name. But can't really think of a better alternative at the moment. As far as implementation, I think building this into the `ConfigBuilder` code would be better. e.g. add a method where you can configure a property name for the "default" value, and when reading a property, both configs would be concatenated with some configurable separator. e.g., using your names: ``` private[spark] val DRIVER_JAVA_OPTIONS = ConfigBuilder(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS) .withDefaultConfig(SparkLauncher.DRIVER_DEFAULT_JAVA_OPTIONS, " ") .stringConf .createOptional ``` And reading `DRIVER_JAVA_OPTIONS` would return the two configs, concatenated, separated by a space. Then you could use this for things like classpath, native libraries, and many others without having to change any of the code that reads those configs. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user squito commented on the issue: https://github.com/apache/spark/pull/20645 I agree it would be nicer to have this be a more general feature. I would prefer an approach which didn't require a different configuration name, just as its more to document & for users to keep track of. An alternative would be to have another syntax for appending to an options, perhaps ++= like scala , eg. "--conf spark.executor.extraJavaOptions++=..." You'd still need to tag the ConfigBuilder with what separator to use to merge the values together. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user rdblue commented on the issue: https://github.com/apache/spark/pull/20645 I like the `ConfigBuilder` approach. That would make this much more useful. I'll add an implementation like that. I think append option syntax would be confusing for users and administrators. We need to make sure it is not easy to overwrite the default options and we can't expect users to know they need to append, and how to do it. Administrators could add `++=` options, but then we would be applying these in the wrong order to get the behavior we're after, which is bad because order matters in some cases. I like the two-property approach better because it is simpler and more predictable. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20645 A quick question: after this change, `extraJavaOptions` is still able to cleanly override whatever's set in `defaultJavaOptions`, is that right? ^^ Just making sure I understood the intent correctly and not the other way around. There may well be the other side of administrative needs which is to "force options", e.g. force `-XX:-DisableExplicitGC` so that NIO direct memory doesn't get into trouble, but that'd be out of scope of this PR. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20645: SPARK-23472: Add defaultJavaOptions for drivers and exec...
Github user rdblue commented on the issue: https://github.com/apache/spark/pull/20645 > A quick question: after this change, extraJavaOptions is still able to cleanly override whatever's set in defaultJavaOptions, is that right? No, the intent is for both sets of options to be passed. How the JVM interprets the options is not up to Spark. This is intended to provide a way for administrators to default properties so they are not accidentally overridden when a user adds `--driver-java-options`. Users can still override `defaultJavaOptions` if they need to deviate from adminstrator defaults. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
