You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Gschiavon <gi...@git.apache.org> on 2017/11/21 16:04:43 UTC
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
GitHub user Gschiavon opened a pull request:
https://github.com/apache/spark/pull/19793
[SPARK-22574] [Mesos] [Submit] Check submission request parameters
## What changes were proposed in this pull request?
It solving the problem when submitting a wrong CreateSubmissionRequest to Spark Dispatcher was causing a bad state of Dispatcher and making it inactive as a Mesos framework.
## How was this patch tested?
It was tested sending a wrong request (without appArgs) before and after the change. The point is easy, check if the value is null before being accessed.
This was before the change, leaving the dispatcher inactive:
```
Exception in thread "Thread-22" java.lang.NullPointerException
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler.getDriverCommandValue(MesosClusterScheduler.scala:444)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler.buildDriverCommand(MesosClusterScheduler.scala:451)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler.org$apache$spark$scheduler$cluster$mesos$MesosClusterScheduler$$createTaskInfo(MesosClusterScheduler.scala:538)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler$$anonfun$scheduleTasks$1.apply(MesosClusterScheduler.scala:570)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler$$anonfun$scheduleTasks$1.apply(MesosClusterScheduler.scala:555)
at scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler.scheduleTasks(MesosClusterScheduler.scala:555)
at org.apache.spark.scheduler.cluster.mesos.MesosClusterScheduler.resourceOffers(MesosClusterScheduler.scala:621)
```
And after:
```
"message" : "Malformed request: org.apache.spark.deploy.rest.SubmitRestProtocolException: Validation of message CreateSubmissionRequest failed!\n\torg.apache.spark.deploy.rest.SubmitRestProtocolMessage.validate(SubmitRestProtocolMessage.scala:70)\n\torg.apache.spark.deploy.rest.SubmitRequestServlet.doPost(RestSubmissionServer.scala:272)\n\tjavax.servlet.http.HttpServlet.service(HttpServlet.java:707)\n\tjavax.servlet.http.HttpServlet.service(HttpServlet.java:790)\n\torg.spark_project.jetty.servlet.ServletHolder.handle(ServletHolder.java:845)\n\torg.spark_project.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:583)\n\torg.spark_project.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1180)\n\torg.spark_project.jetty.servlet.ServletHandler.doScope(ServletHandler.java:511)\n\torg.spark_project.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112)\n\torg.spark_project.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)\n\torg.s
park_project.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)\n\torg.spark_project.jetty.server.Server.handle(Server.java:524)\n\torg.spark_project.jetty.server.HttpChannel.handle(HttpChannel.java:319)\n\torg.spark_project.jetty.server.HttpConnection.onFillable(HttpConnection.java:253)\n\torg.spark_project.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:273)\n\torg.spark_project.jetty.io.FillInterest.fillable(FillInterest.java:95)\n\torg.spark_project.jetty.io.SelectChannelEndPoint$2.run(SelectChannelEndPoint.java:93)\n\torg.spark_project.jetty.util.thread.strategy.ExecuteProduceConsume.executeProduceConsume(ExecuteProduceConsume.java:303)\n\torg.spark_project.jetty.util.thread.strategy.ExecuteProduceConsume.produceConsume(ExecuteProduceConsume.java:148)\n\torg.spark_project.jetty.util.thread.strategy.ExecuteProduceConsume.run(ExecuteProduceConsume.java:136)\n\torg.spark_project.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.
java:671)\n\torg.spark_project.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:589)\n\tjava.lang.Thread.run(Thread.java:745)",
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Gschiavon/spark fix-submission-request
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19793.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 #19793
----
commit 4f28d0011892326c6ec3fd9a4fc6fb48756cd635
Author: German Schiavon <ge...@gmail.com>
Date: 2017-11-21T15:32:04Z
Check submission request parameters
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153410771
--- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala ---
@@ -82,6 +82,12 @@ private[mesos] class MesosSubmitRequestServlet(
val mainClass = Option(request.mainClass).getOrElse {
throw new SubmitRestMissingFieldException("Main class is missing.")
}
+ val appArgs = Option(request.appArgs).getOrElse {
+ throw new SubmitRestMissingFieldException("Application arguments are missing.")
--- End diff --
Done @ArtRand
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19793
**[Test build #3993 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3993/testReport)** for PR 19793 at commit [`14d6417`](https://github.com/apache/spark/commit/14d64172500483f9e984ac28ba5f3b52db33ad9e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19793
yea please open a new PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153824692
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
OK, I was looking at RestSubmissionClient (used by Mesos and Standalone) - the client does always set 'appArgs' and 'envrionmentVariables': https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/rest/RestSubmissionClient.scala#L427 So, it's only a bug when the user does not use the RestSubmissionClient (by using 'curl' directly to the server, for example). So, that addresses my concern about Standalone mode. As to whether to add a similar check in the Standalone class, I don't have a strong opinion about it (fix it here or fix in another PR).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153352732
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
There should be a check at the end of this test to make sure these fields show up in the `newMessage` object. (around L.125)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153080692
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_SCALA_VERSION" -> "2.11")
--- End diff --
we might be able to set this here - we need to have it working with multiple versions of scala
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153906029
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
Yeah, that's fine with me.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153352098
--- Diff: core/src/main/scala/org/apache/spark/deploy/rest/SubmitRestProtocolRequest.scala ---
@@ -46,6 +46,8 @@ private[rest] class CreateSubmissionRequest extends SubmitRestProtocolRequest {
super.doValidate()
assert(sparkProperties != null, "No Spark properties set!")
assertFieldIsSet(appResource, "appResource")
+ assertFieldIsSet(appArgs, "appArgs")
+ assertFieldIsSet(environmentVariables, "environmentVariables")
--- End diff --
What if there are no args or environment variables for a particular job? Is the caller expected to pass in an empty array?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19793
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r154991966
--- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala ---
@@ -77,10 +77,16 @@ private[mesos] class MesosSubmitRequestServlet(
private def buildDriverDescription(request: CreateSubmissionRequest): MesosDriverDescription = {
// Required fields, including the main class because python is not yet supported
val appResource = Option(request.appResource).getOrElse {
- throw new SubmitRestMissingFieldException("Application jar is missing.")
+ throw new SubmitRestMissingFieldException("Application jar 'appResource' is missing.")
}
val mainClass = Option(request.mainClass).getOrElse {
- throw new SubmitRestMissingFieldException("Main class is missing.")
+ throw new SubmitRestMissingFieldException("Main class 'mainClass' is missing.")
+ }
+ val appArgs = Option(request.appArgs).getOrElse {
+ throw new SubmitRestMissingFieldException("Application arguments 'appArgs' are missing.")
+ }
+ val environmentVariables = Option(request.environmentVariables).getOrElse {
+ throw new SubmitRestMissingFieldException("Environment variables 'environmentVariables' are missing.")
--- End diff --
Yes, the arguments were assumed to be there, but validation was missing.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on the issue:
https://github.com/apache/spark/pull/19793
Hi @vanzin ! I just fixed it and pushed it to my branch but it's not opening the PR, maybe I need to do a new one?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153901749
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
Perfect then @susanxhuynh . Fix it maybe in another PR?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/19793
Jenkins, test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19793
**[Test build #84197 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84197/testReport)** for PR 19793 at commit [`44fd5d3`](https://github.com/apache/spark/commit/44fd5d3921299f93d1aab7fe971078a6bce835a2).
* This patch **fails Spark unit 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 pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153413536
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
and probably don't want to set SPARK_HOME either? :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153415074
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
@susanxhuynh I've checked what you said but those variables are overwritten below, so I can't actually check it, right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153414519
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
@felixcheung okay done :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on the issue:
https://github.com/apache/spark/pull/19793
ping @ArtRand
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19793
Done. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19793
Merging to master / 2.2.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r154102830
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153530156
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
@Gschiavon You're right, I didn't see those variables were already set below. But, I do have a question now about whether these fields are optional or not. In the original test, (L.93), there's a comment about "optional fields", and the app args and env vars are tested there (L.105-106). Since this test was added with Standalone mode in mind, I wonder if these fields are considered optional there? There's no documentation as you said so we can't know for sure. I just want to make sure the extra validation doesn't break any existing applications (in Standalone cluster mode).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153709924
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
@susanxhuynh Yes we have to make sure we don't break the Standalone mode.
I've reviewed StandaloneSubmitRequestServlet class and i think we might be facing the same problem here, (L.126-131) You can find checks for 'appResource' and 'mainClass' but not for 'appArgs'(L.140) or 'environmentVariables'(L.143) when they could be null as they are initialised as null.
I think is the same case, let me know what you think.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19793
**[Test build #84197 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84197/testReport)** for PR 19793 at commit [`44fd5d3`](https://github.com/apache/spark/commit/44fd5d3921299f93d1aab7fe971078a6bce835a2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19793
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19793
**[Test build #3993 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3993/testReport)** for PR 19793 at commit [`14d6417`](https://github.com/apache/spark/commit/14d64172500483f9e984ac28ba5f3b52db33ad9e).
* 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 #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on the issue:
https://github.com/apache/spark/pull/19793
@Gschiavon Looking.. thanks for the ping.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r154072747
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_HOME" -> "/test")
--- End diff --
Cool, I can do it when I have some time.
Let me know if I have to something else here!
Thanks @susanxhuynh
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153348394
--- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala ---
@@ -82,6 +82,12 @@ private[mesos] class MesosSubmitRequestServlet(
val mainClass = Option(request.mainClass).getOrElse {
throw new SubmitRestMissingFieldException("Main class is missing.")
}
+ val appArgs = Option(request.appArgs).getOrElse {
+ throw new SubmitRestMissingFieldException("Application arguments are missing.")
--- End diff --
Nit: maybe put the fields here also? Something like `Application arguments ("appArgs") are missing`. Similar below.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19793
(Just saw you did it, thanks.)
@Gschiavon please fix the issue and push to your branch (that will re-open the PR).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19793
My bad, did you revert it already? Otherwise I'll do it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r154859418
--- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala ---
@@ -77,10 +77,16 @@ private[mesos] class MesosSubmitRequestServlet(
private def buildDriverDescription(request: CreateSubmissionRequest): MesosDriverDescription = {
// Required fields, including the main class because python is not yet supported
val appResource = Option(request.appResource).getOrElse {
- throw new SubmitRestMissingFieldException("Application jar is missing.")
+ throw new SubmitRestMissingFieldException("Application jar 'appResource' is missing.")
}
val mainClass = Option(request.mainClass).getOrElse {
- throw new SubmitRestMissingFieldException("Main class is missing.")
+ throw new SubmitRestMissingFieldException("Main class 'mainClass' is missing.")
+ }
+ val appArgs = Option(request.appArgs).getOrElse {
+ throw new SubmitRestMissingFieldException("Application arguments 'appArgs' are missing.")
+ }
+ val environmentVariables = Option(request.environmentVariables).getOrElse {
+ throw new SubmitRestMissingFieldException("Environment variables 'environmentVariables' are missing.")
--- End diff --
so this is API changes - new required arguments for the request.
are folks ok with this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/19793
@vanzin It seems this PR breaks [Jenkins test](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84790/console), could you help to resolve it? Also cc @gatorsmile @cloud-fan. Thanks.
```
Running Scala style checks
========================================================================
Scalastyle checks failed at following occurrences:
[error] /home/jenkins/workspace/SparkPullRequestBuilder@2/resource-managers/mesos/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala:89: File line length exceeds 100 characters
[error] Total time: 17 s, completed Dec 12, 2017 1:29:36 PM
[error] running /home/jenkins/workspace/SparkPullRequestBuilder@2/dev/lint-scala ; received return code 1
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on the issue:
https://github.com/apache/spark/pull/19793
can we retest this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153131125
--- Diff: core/src/test/scala/org/apache/spark/deploy/rest/SubmitRestProtocolSuite.scala ---
@@ -86,6 +86,8 @@ class SubmitRestProtocolSuite extends SparkFunSuite {
message.clientSparkVersion = "1.2.3"
message.appResource = "honey-walnut-cherry.jar"
message.mainClass = "org.apache.spark.examples.SparkPie"
+ message.appArgs = Array("hdfs://tmp/auth")
+ message.environmentVariables = Map("SPARK_SCALA_VERSION" -> "2.11")
--- End diff --
In this case it's just a name, I could change it in order to make it clear!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on the issue:
https://github.com/apache/spark/pull/19793
Yes @ArtRand! I think it's not documented at all, ping me when it's done and I will review it :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19793
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19793: [SPARK-22574] [Mesos] [Submit] Check submission r...
Posted by Gschiavon <gi...@git.apache.org>.
Github user Gschiavon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19793#discussion_r153410217
--- Diff: core/src/main/scala/org/apache/spark/deploy/rest/SubmitRestProtocolRequest.scala ---
@@ -46,6 +46,8 @@ private[rest] class CreateSubmissionRequest extends SubmitRestProtocolRequest {
super.doValidate()
assert(sparkProperties != null, "No Spark properties set!")
assertFieldIsSet(appResource, "appResource")
+ assertFieldIsSet(appArgs, "appArgs")
+ assertFieldIsSet(environmentVariables, "environmentVariables")
--- End diff --
Actually If the caller wouldn't set "appArgs" or "environmentVariables" was causing a null pointer and leaving the Dispatcher inactive. So now I think the caller should pass an empty array, I could add a test for that case @susanxhuynh .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19793
Sorry, I need to revert this PR. This was merged without triggering a test. It breaks the builds
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19793: [SPARK-22574] [Mesos] [Submit] Check submission request ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19793
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84197/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org