You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by vanzin <gi...@git.apache.org> on 2017/10/17 18:58:39 UTC
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
GitHub user vanzin opened a pull request:
https://github.com/apache/spark/pull/19519
[SPARK-21840][core] Add trait that allows conf to be directly set in application.
Currently SparkSubmit uses system properties to propagate configuration to
applications. This makes it hard to implement features such as SPARK-11035,
which would allow multiple applications to be started in the same JVM. The
current code would cause the config data from multiple apps to get mixed
up.
This change introduces a new trait, currently internal to Spark, that allows
the app configuration to be passed directly to the application, without
having to use system properties. The current "call main() method" behavior
is maintained as an implementation of this new trait. This will be useful
to allow multiple cluster mode apps to be submitted from the same JVM.
As part of this, SparkSubmit was modified to collect all configuration
directly into a SparkConf instance. Most of the changes are to tests so
they use SparkConf instead of an opaque map.
Tested with existing and added unit tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vanzin/spark SPARK-21840
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19519.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19519
----
commit 8f31cf5ed7b22cb625e29c6dc0b8a01ff73cd8e1
Author: Marcelo Vanzin <va...@cloudera.com>
Date: 2017-10-17T17:21:50Z
[SPARK-21840][core] Add trait that allows conf to be directly set in application.
Currently SparkSubmit uses system properties to propagate configuration to
applications. This makes it hard to implement features such as SPARK-11035,
which would allow multiple applications to be started in the same JVM. The
current code would cause the config data from multiple apps to get mixed
up.
This change introduces a new trait, currently internal to Spark, that allows
the app configuration to be passed directly to the application, without
having to use system properties. The current "call main() method" behavior
is maintained as an implementation of this new trait. This will be useful
to allow multiple cluster mode apps to be submitted from the same JVM.
As part of this, SparkSubmit was modified to collect all configuration
directly into a SparkConf instance. Most of the changes are to tests so
they use SparkConf instead of an opaque map.
Tested with existing and added unit tests.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r146332249
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkApplication.scala ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy
+
+import java.lang.reflect.Modifier
+
+import org.apache.spark.SparkConf
+
+/**
+ * Entry point for a Spark application. Implementations must provide a no-argument constructor.
+ */
+private[spark] trait SparkApplication {
+
+ def start(args: Array[String], conf: SparkConf): Unit
+
+}
+
+/**
+ * Implementation of SparkApplication that wraps a standard Java class with a "main" method.
+ *
+ * Configuration is propagated to the application via system properties, so running multiple
+ * of these in the same JVM may lead to undefined behavior due to configuration leaks.
+ */
+private[deploy] class JavaMainApplication(klass: Class[_]) extends SparkApplication {
+
+ override def start(args: Array[String], conf: SparkConf): Unit = {
+ val mainMethod = klass.getMethod("main", new Array[String](0).getClass)
+ if (!Modifier.isStatic(mainMethod.getModifiers)) {
+ throw new IllegalStateException("The main method in the given main class must be static")
+ }
+
+ val sysProps = conf.getAll.toMap
+ sysProps.foreach { case (k, v) =>
+ sys.props(k) = v
+ }
+
+ mainMethod.invoke(null, args)
+ }
--- End diff --
That's dangerous, because you don't know what the user code is doing. What if it spawn a thread and that separate thread creates the `SparkContext`? Then you may be clearing system properties before the user app had the chance to read them.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82852/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r145492860
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -235,11 +235,11 @@ object SparkSubmit extends CommandLineUtils with Logging {
private[deploy] def prepareSubmitEnvironment(
args: SparkSubmitArguments,
conf: Option[HadoopConfiguration] = None)
- : (Seq[String], Seq[String], Map[String, String], String) = {
+ : (Seq[String], Seq[String], SparkConf, String) = {
// Return values
val childArgs = new ArrayBuffer[String]()
val childClasspath = new ArrayBuffer[String]()
- val sysProps = new HashMap[String, String]()
+ val sparkConf = new SparkConf()
--- End diff --
Yes. Becase this conf will now be exposed to apps (once I change code to extend `SparkApplication`), the conf needs to respect system properties.
In fact the previous version should probably have done that too from the get go.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82863 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82863/testReport)** for PR 19519 at commit [`d4466f2`](https://github.com/apache/spark/commit/d4466f269451ef23d9c4d45fcd5b1c03988ac0e7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19519
> how do we leverage this new trait
In a separate future commit I'll change the YARN Client, for example, to extend from this new trait. That will allow multiple submissions of yarn-cluster apps from the same JVM without getting configuration options being mixed up.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82992 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82992/testReport)** for PR 19519 at commit [`1915135`](https://github.com/apache/spark/commit/191513542da1c11f66dd96cca472babe68b4f630).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `final class OnlineLDAOptimizer extends LDAOptimizer with Logging `
* `class PythonUdfType(object):`
* `case class WriteToDataSourceV2(writer: DataSourceV2Writer, query: LogicalPlan) extends LogicalPlan `
* `case class WriteToDataSourceV2Exec(writer: DataSourceV2Writer, query: SparkPlan) extends SparkPlan `
* `class RowToInternalRowDataWriterFactory(`
* `class RowToInternalRowDataWriter(rowWriter: DataWriter[Row], encoder: ExpressionEncoder[Row])`
* ` case class JoinConditionSplitPredicates(`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19519
@vanzin , how do we leverage this new trait, would you please explain more? Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r146737263
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkApplication.scala ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy
+
+import java.lang.reflect.Modifier
+
+import org.apache.spark.SparkConf
+
+/**
+ * Entry point for a Spark application. Implementations must provide a no-argument constructor.
+ */
+private[spark] trait SparkApplication {
+
+ def start(args: Array[String], conf: SparkConf): Unit
+
+}
+
+/**
+ * Implementation of SparkApplication that wraps a standard Java class with a "main" method.
+ *
+ * Configuration is propagated to the application via system properties, so running multiple
+ * of these in the same JVM may lead to undefined behavior due to configuration leaks.
+ */
+private[deploy] class JavaMainApplication(klass: Class[_]) extends SparkApplication {
+
+ override def start(args: Array[String], conf: SparkConf): Unit = {
+ val mainMethod = klass.getMethod("main", new Array[String](0).getClass)
+ if (!Modifier.isStatic(mainMethod.getModifiers)) {
+ throw new IllegalStateException("The main method in the given main class must be static")
+ }
+
+ val sysProps = conf.getAll.toMap
+ sysProps.foreach { case (k, v) =>
+ sys.props(k) = v
+ }
+
+ mainMethod.invoke(null, args)
+ }
--- End diff --
I see, thanks for explanation. I cannot figure out a solution which doesn't break the current semantics of `SparkConf`, this might be the only choice.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r145495282
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -235,11 +235,11 @@ object SparkSubmit extends CommandLineUtils with Logging {
private[deploy] def prepareSubmitEnvironment(
args: SparkSubmitArguments,
conf: Option[HadoopConfiguration] = None)
- : (Seq[String], Seq[String], Map[String, String], String) = {
+ : (Seq[String], Seq[String], SparkConf, String) = {
// Return values
val childArgs = new ArrayBuffer[String]()
val childClasspath = new ArrayBuffer[String]()
- val sysProps = new HashMap[String, String]()
+ val sparkConf = new SparkConf()
--- End diff --
Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82992/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82852 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82852/testReport)** for PR 19519 at commit [`8f31cf5`](https://github.com/apache/spark/commit/8f31cf5ed7b22cb625e29c6dc0b8a01ff73cd8e1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19519
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r146734075
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkApplication.scala ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy
+
+import java.lang.reflect.Modifier
+
+import org.apache.spark.SparkConf
+
+/**
+ * Entry point for a Spark application. Implementations must provide a no-argument constructor.
+ */
+private[spark] trait SparkApplication {
+
+ def start(args: Array[String], conf: SparkConf): Unit
+
+}
+
+/**
+ * Implementation of SparkApplication that wraps a standard Java class with a "main" method.
+ *
+ * Configuration is propagated to the application via system properties, so running multiple
+ * of these in the same JVM may lead to undefined behavior due to configuration leaks.
+ */
+private[deploy] class JavaMainApplication(klass: Class[_]) extends SparkApplication {
+
+ override def start(args: Array[String], conf: SparkConf): Unit = {
+ val mainMethod = klass.getMethod("main", new Array[String](0).getClass)
+ if (!Modifier.isStatic(mainMethod.getModifiers)) {
+ throw new IllegalStateException("The main method in the given main class must be static")
+ }
+
+ val sysProps = conf.getAll.toMap
+ sysProps.foreach { case (k, v) =>
+ sys.props(k) = v
+ }
+
+ mainMethod.invoke(null, args)
+ }
--- End diff --
But based on your comment "allow multiple applications to be started in the same JVM", will this system properties contaminate follow-up applications? Sorry if I misunderstood your scenario.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r146154530
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkApplication.scala ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy
+
+import java.lang.reflect.Modifier
+
+import org.apache.spark.SparkConf
+
+/**
+ * Entry point for a Spark application. Implementations must provide a no-argument constructor.
+ */
+private[spark] trait SparkApplication {
+
+ def start(args: Array[String], conf: SparkConf): Unit
+
+}
+
+/**
+ * Implementation of SparkApplication that wraps a standard Java class with a "main" method.
+ *
+ * Configuration is propagated to the application via system properties, so running multiple
+ * of these in the same JVM may lead to undefined behavior due to configuration leaks.
+ */
+private[deploy] class JavaMainApplication(klass: Class[_]) extends SparkApplication {
+
+ override def start(args: Array[String], conf: SparkConf): Unit = {
+ val mainMethod = klass.getMethod("main", new Array[String](0).getClass)
+ if (!Modifier.isStatic(mainMethod.getModifiers)) {
+ throw new IllegalStateException("The main method in the given main class must be static")
+ }
+
+ val sysProps = conf.getAll.toMap
+ sysProps.foreach { case (k, v) =>
+ sys.props(k) = v
+ }
+
+ mainMethod.invoke(null, args)
+ }
--- End diff --
@vanzin , do we need to remove all the system properties after `mainMethod` is finished?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19519
LGTM.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82863/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82852 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82852/testReport)** for PR 19519 at commit [`8f31cf5`](https://github.com/apache/spark/commit/8f31cf5ed7b22cb625e29c6dc0b8a01ff73cd8e1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` throw new IllegalStateException(\"The main method in the given main class must be static\")`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19519
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r145491446
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -235,11 +235,11 @@ object SparkSubmit extends CommandLineUtils with Logging {
private[deploy] def prepareSubmitEnvironment(
args: SparkSubmitArguments,
conf: Option[HadoopConfiguration] = None)
- : (Seq[String], Seq[String], Map[String, String], String) = {
+ : (Seq[String], Seq[String], SparkConf, String) = {
// Return values
val childArgs = new ArrayBuffer[String]()
val childClasspath = new ArrayBuffer[String]()
- val sysProps = new HashMap[String, String]()
+ val sparkConf = new SparkConf()
--- End diff --
Hi, @vanzin .
Is it intentionally loading default config? Previously, it wasn't in [line 340](https://github.com/apache/spark/pull/19519/files#diff-4d2ab44195558d5a9d5f15b8803ef39dL340).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82863 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82863/testReport)** for PR 19519 at commit [`d4466f2`](https://github.com/apache/spark/commit/d4466f269451ef23d9c4d45fcd5b1c03988ac0e7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19519
LGTM, merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19519: [SPARK-21840][core] Add trait that allows conf to be dir...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19519
**[Test build #82992 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82992/testReport)** for PR 19519 at commit [`1915135`](https://github.com/apache/spark/commit/191513542da1c11f66dd96cca472babe68b4f630).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19519: [SPARK-21840][core] Add trait that allows conf to...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19519#discussion_r146734568
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkApplication.scala ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy
+
+import java.lang.reflect.Modifier
+
+import org.apache.spark.SparkConf
+
+/**
+ * Entry point for a Spark application. Implementations must provide a no-argument constructor.
+ */
+private[spark] trait SparkApplication {
+
+ def start(args: Array[String], conf: SparkConf): Unit
+
+}
+
+/**
+ * Implementation of SparkApplication that wraps a standard Java class with a "main" method.
+ *
+ * Configuration is propagated to the application via system properties, so running multiple
+ * of these in the same JVM may lead to undefined behavior due to configuration leaks.
+ */
+private[deploy] class JavaMainApplication(klass: Class[_]) extends SparkApplication {
+
+ override def start(args: Array[String], conf: SparkConf): Unit = {
+ val mainMethod = klass.getMethod("main", new Array[String](0).getClass)
+ if (!Modifier.isStatic(mainMethod.getModifiers)) {
+ throw new IllegalStateException("The main method in the given main class must be static")
+ }
+
+ val sysProps = conf.getAll.toMap
+ sysProps.foreach { case (k, v) =>
+ sys.props(k) = v
+ }
+
+ mainMethod.invoke(null, args)
+ }
--- End diff --
Yes, and there's not much we can do in that case. The main use case will be to make cluster mode applications do the right thing first, and warn about starting in-process client mode applications through the launcher API.
If you have a better solution I'm all ears.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org