You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by skonto <gi...@git.apache.org> on 2018/05/20 19:54:22 UTC

[GitHub] spark pull request #21378: [SPARK-24326][mesos] add support for local:// sch...

GitHub user skonto opened a pull request:

    https://github.com/apache/spark/pull/21378

    [SPARK-24326][mesos] add support for local:// scheme for the app jar

    ## What changes were proposed in this pull request?
    
    *Adds support for local:// scheme like in k8s case for image based deployments where the jar is already in the image. Affects cluster mode and the mesos dispatcher. Mimics the k8s approach. Covers also file:// scheme. Keeps the default case where jar resolution happens on the host.
    
    ## How was this patch tested?
    
    ispatcher image with the patch, use it to start DC/OS Spark service: 
    skonto/spark-local-disp:test
    
    Test image with my application jar located at the root folder:
    skonto/spark-local:test
    
    Dockerfile for that image.
    
    From mesosphere/spark:2.3.0-2.2.1-2-hadoop-2.6
    COPY spark-examples_2.11-2.2.1.jar /
    WORKDIR /opt/spark/dist
    
    Tests:
    
    The following work as expected:
    
    * local normal example
    ```
    dcos spark run --submit-args="--conf spark.mesos.appJar.local.resolution.mode=container --conf spark.executor.memory=1g --conf spark.mesos.executor.docker.image=skonto/spark-local:test
     --conf spark.executor.cores=2 --conf spark.cores.max=8
     --class org.apache.spark.examples.SparkPi local:///spark-examples_2.11-2.2.1.jar"
    ```
    
    * make sure the flag does not affect other uris
    ```
    dcos spark run --submit-args="--conf spark.mesos.appJar.local.resolution.mode=container --conf spark.executor.memory=1g  --conf spark.executor.cores=2 --conf spark.cores.max=8
     --class org.apache.spark.examples.SparkPi https://s3-eu-west-1.amazonaws.com/fdp-stavros-test/spark-examples_2.11-2.1.1.jar"
    ```
    
    * normal example no local
    ```
    dcos spark run --submit-args="--conf spark.executor.memory=1g  --conf spark.executor.cores=2 --conf spark.cores.max=8
     --class org.apache.spark.examples.SparkPi https://s3-eu-west-1.amazonaws.com/fdp-stavros-test/spark-examples_2.11-2.1.1.jar"
    
    ```
    
    The following fails
    
     * uses local with no setting, default is host.
    ```
    dcos spark run --submit-args="--conf spark.executor.memory=1g --conf spark.mesos.executor.docker.image=skonto/spark-local:test
      --conf spark.executor.cores=2 --conf spark.cores.max=8
      --class org.apache.spark.examples.SparkPi local:///spark-examples_2.11-2.2.1.jar"
    ```
    ![image](https://user-images.githubusercontent.com/7945591/40283021-8d349762-5c80-11e8-9d62-2a61a4318fd5.png)
    
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/skonto/spark local-upstream

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/21378.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 #21378
    
----
commit e379d92768c6f0ed3eb7f359f9bdbd2313a1705e
Author: Stavros Kontopoulos <st...@...>
Date:   2018-05-20T19:50:20Z

    add support for local:// scheme for app jar

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @felixcheung yup. I was just looking at this PR out of my curiosity. I don't currently have an env to test Mesos.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191166429
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,34 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    --- End diff --
    
    Not really. Option.exists() returns false when there the option has vale None.
    Other describes an arbitrary string here. Target here is to return a Boolean value and exists does just that under certain conditions. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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/3639/
    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 #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191449756
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    +  <td><code>spark.mesos.appJar.local.resolution.mode</code></td>
    +  <td><code>host</code></td>
    +  <td>
    +  Provides support for the local:/// scheme to reference the app jar resource in cluster mode.
    +  If user uses a local resource (local:///path/to/jar) and the config option is not used it defaults to `host` eg. the mesos fetcher tries to get the resource from the host's file system.
    +  If the value is unknown it prints a warning msg in the dispatcher logs and defaults to `host`.
    +  If the value is `container` then spark submit in the container will use the jar in the container's path: /path/to/jar.
    +  </td>
    --- End diff --
    
    Ok I havent built the docs, will update it.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91224/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #90866 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90866/testReport)** for PR 21378 at commit [`e379d92`](https://github.com/apache/spark/commit/e379d92768c6f0ed3eb7f359f9bdbd2313a1705e).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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/3705/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91337 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91337/testReport)** for PR 21378 at commit [`07e2210`](https://github.com/apache/spark/commit/07e221090560a924b2a5dec4fd91384ce5ba8c7b).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91304/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #90866 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90866/testReport)** for PR 21378 at commit [`e379d92`](https://github.com/apache/spark/commit/e379d92768c6f0ed3eb7f359f9bdbd2313a1705e).
     * 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 pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191056648
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    +  <td><code>spark.mesos.appJar.local.resolution.mode</code></td>
    +  <td><code>host</code></td>
    +  <td>
    +  Provides support for the local:/// scheme to reference the app jar resource in cluster mode.
    +  If user uses a local resource (local:///path/to/jar) and the config option is not used it defaults to `host` eg. the mesos fetcher tries to get the resource from the host's file system.
    +  If the value is unknown it prints a warning msg in the dispatcher logs and defaults to host.
    +  If the user uses container value then spark submit in the container will use the jar in the container's path: /path/to/jar.
    --- End diff --
    
    let's try to be consistent? instead of `If the user uses container value` -> `If the value is container`
    
    also please use backtick around acceptable values like `host`, `container`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91304 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91304/testReport)** for PR 21378 at commit [`ef2de02`](https://github.com/apache/spark/commit/ef2de023b5d3071c96189ead60abbe81035077c1).
     * 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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91224 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91224/testReport)** for PR 21378 at commit [`a6a308b`](https://github.com/apache/spark/commit/a6a308b68cfeab138f27cba1c0cc78a8aa50fc6e).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @felixcheung I think its ready now ;)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191724625
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,33 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    --- End diff --
    
    Ok I see.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191449547
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,33 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    --- End diff --
    
    What's wrong with exists? 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191166695
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,34 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    +    isLocalJar && isContainerLocal
    +  }
    +
       private def getDriverUris(desc: MesosDriverDescription): List[CommandInfo.URI] = {
         val confUris = List(conf.getOption("spark.mesos.uris"),
           desc.conf.getOption("spark.mesos.uris"),
           desc.conf.getOption("spark.submit.pyFiles")).flatMap(
           _.map(_.split(",").map(_.trim))
         ).flatten
     
    -    val jarUrl = desc.jarUrl.stripPrefix("file:").stripPrefix("local:")
    -
    -    ((jarUrl :: confUris) ++ getDriverExecutorURI(desc).toList).map(uri =>
    -      CommandInfo.URI.newBuilder().setValue(uri.trim()).setCache(useFetchCache).build())
    +    if (isContainerLocalAppJar(desc)) {
    +      (confUris ++ getDriverExecutorURI(desc).toList).map(uri =>
    +        CommandInfo.URI.newBuilder().setValue(uri.trim()).setCache(useFetchCache).build())
    +    }
    +    else {
    --- End diff --
    
    Sure.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191630562
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,33 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    --- End diff --
    
    Nothing wrong but I just find it hard to read. I assume @felixcheung had a similar concern at the core.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91341 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91341/testReport)** for PR 21378 at commit [`e7e794a`](https://github.com/apache/spark/commit/e7e794a69bb362fc04b36fb93c7ab7a06aeec4ce).
     * 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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    merged to master. thanks!


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91341 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91341/testReport)** for PR 21378 at commit [`e7e794a`](https://github.com/apache/spark/commit/e7e794a69bb362fc04b36fb93c7ab7a06aeec4ce).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91339 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91339/testReport)** for PR 21378 at commit [`e3055c3`](https://github.com/apache/spark/commit/e3055c3f2a70671eefad85391da712455df07273).
     * 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 pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191647592
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,33 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    --- End diff --
    
    It's not very commonly used, but ok to me.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91224 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91224/testReport)** for PR 21378 at commit [`a6a308b`](https://github.com/apache/spark/commit/a6a308b68cfeab138f27cba1c0cc78a8aa50fc6e).
     * 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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @felixcheung ready :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191310691
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,33 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    --- End diff --
    
    Can we do:
    
    ```scala
    desc.conf.getOption("spark.mesos.appJar.local.resolution.mode") match {
        case Some("container") => true
        case Some("host") | None => false
        case Some(other) =>
            ...
    }
    ```
    ?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @felixcheung could you review pls?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test FAILed.
    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/3728/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Thank you @susanxhuynh 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191070427
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,34 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    +      case "container" => true
    +      case "host" => false
    +      case other =>
    +        logWarning(s"Unknown spark.mesos.appJar.local.resolution.mode $other, using host.")
    +        false
    +      }
    +    isLocalJar && isContainerLocal
    +  }
    +
       private def getDriverUris(desc: MesosDriverDescription): List[CommandInfo.URI] = {
         val confUris = List(conf.getOption("spark.mesos.uris"),
           desc.conf.getOption("spark.mesos.uris"),
           desc.conf.getOption("spark.submit.pyFiles")).flatMap(
           _.map(_.split(",").map(_.trim))
         ).flatten
     
    -    val jarUrl = desc.jarUrl.stripPrefix("file:").stripPrefix("local:")
    -
    -    ((jarUrl :: confUris) ++ getDriverExecutorURI(desc).toList).map(uri =>
    -      CommandInfo.URI.newBuilder().setValue(uri.trim()).setCache(useFetchCache).build())
    +    if (isContainerLocalAppJar(desc)) {
    +      (confUris ++ getDriverExecutorURI(desc).toList).map(uri =>
    +        CommandInfo.URI.newBuilder().setValue(uri.trim()).setCache(useFetchCache).build())
    +    }
    +    else {
    --- End diff --
    
    nit: ->  `} else {`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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/3396/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @susanxhuynh pls review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91338 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91338/testReport)** for PR 21378 at commit [`07fdeff`](https://github.com/apache/spark/commit/07fdeff662d98391b906e7f549b2534d54657f60).
     * 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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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/3731/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91341/
    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 #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191969654
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    --- End diff --
    
    +1 for reverting whitespaces. Let's leave this minimised and targeted.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90866/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191272157
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    +  <td><code>spark.mesos.appJar.local.resolution.mode</code></td>
    +  <td><code>host</code></td>
    +  <td>
    +  Provides support for the local:/// scheme to reference the app jar resource in cluster mode.
    +  If user uses a local resource (local:///path/to/jar) and the config option is not used it defaults to `host` eg. the mesos fetcher tries to get the resource from the host's file system.
    +  If the value is unknown it prints a warning msg in the dispatcher logs and defaults to `host`.
    +  If the value is `container` then spark submit in the container will use the jar in the container's path: /path/to/jar.
    +  </td>
    --- End diff --
    
    I think all `local:/` and `/path/to` should be insider backtick, otherwise it might not render html properly?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91304/testReport)** for PR 21378 at commit [`ef2de02`](https://github.com/apache/spark/commit/ef2de023b5d3071c96189ead60abbe81035077c1).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191842162
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    --- End diff --
    
    this has an extra `+<tr>`
    
    also if you are at it could you revert the other whitespace changes above?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91338/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Thnx @felixcheung !


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r192061471
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    --- End diff --
    
    Ok sure.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    @felixcheung @HyukjinKwon thanx for the review. I updated the PR.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91337 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91337/testReport)** for PR 21378 at commit [`07e2210`](https://github.com/apache/spark/commit/07e221090560a924b2a5dec4fd91384ce5ba8c7b).
     * 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 #21378: [SPARK-24326][mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91339 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91339/testReport)** for PR 21378 at commit [`e3055c3`](https://github.com/apache/spark/commit/e3055c3f2a70671eefad85391da712455df07273).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91339/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    **[Test build #91338 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91338/testReport)** for PR 21378 at commit [`07fdeff`](https://github.com/apache/spark/commit/07fdeff662d98391b906e7f549b2534d54657f60).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test FAILed.
    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/3729/
    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 #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191166657
  
    --- Diff: docs/running-on-mesos.md ---
    @@ -753,6 +753,16 @@ See the [configuration page](configuration.html) for information on Spark config
         <code>spark.cores.max</code> is reached
       </td>
     </tr>
    ++<tr>
    +  <td><code>spark.mesos.appJar.local.resolution.mode</code></td>
    +  <td><code>host</code></td>
    +  <td>
    +  Provides support for the local:/// scheme to reference the app jar resource in cluster mode.
    +  If user uses a local resource (local:///path/to/jar) and the config option is not used it defaults to `host` eg. the mesos fetcher tries to get the resource from the host's file system.
    +  If the value is unknown it prints a warning msg in the dispatcher logs and defaults to host.
    +  If the user uses container value then spark submit in the container will use the jar in the container's path: /path/to/jar.
    --- End diff --
    
    Sure.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/21378


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91337/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21378: [SPARK-24326][Mesos] add support for local:// scheme for...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21378
  
    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 #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191271894
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,34 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    --- End diff --
    
    interesting! 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21378: [SPARK-24326][Mesos] add support for local:// sch...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21378#discussion_r191056976
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -418,17 +417,34 @@ private[spark] class MesosClusterScheduler(
         envBuilder.build()
       }
     
    +  private def isContainerLocalAppJar(desc: MesosDriverDescription): Boolean = {
    +    val isLocalJar = desc.jarUrl.startsWith("local://")
    +    val isContainerLocal = desc.conf.getOption("spark.mesos.appJar.local.resolution.mode").exists {
    --- End diff --
    
    instead of `.exists` shouldn't this be `.flatmap`?
    
    also does `case other` handles the case when the option is unset?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org