You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by liyinan926 <gi...@git.apache.org> on 2018/02/08 22:36:00 UTC

[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

GitHub user liyinan926 opened a pull request:

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

    [SPARK-23285][K8S] Add a config property for specifying physical executor cores

    As discussed in SPARK-23285, this PR introduces a new configuation property `spark.kubernetes.executor.cores` for specifying the phyiscal CPU cores requested for each executor pod. This is to avoid changing the semantics of `spark.executor.cores` and `spark.task.cpus` and their role in task scheduling, task parallelism, dynamic resource allocation, etc. The new configuraiton property only determines the physical CPU cores available to an executor. An executor can still run multiple tasks simultaneously by using appropriate values for `spark.executor.cores` and `spark.task.cpus`.
    
    ## What changes were proposed in this pull request?
    
    (Please fill in changes proposed in this fix)
    
    ## How was this patch tested?
    
    (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
    (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


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

    $ git pull https://github.com/liyinan926/spark-k8s master

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

    https://github.com/apache/spark/pull/20553.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 #20553
    
----
commit 50ebb5068a35a9a0f2becd27153bdc7cc7aae251
Author: Yinan Li <li...@...>
Date:   2018-02-08T22:22:46Z

    [SPARK-23285][K8S] Add a config property for specifying physical executor cores
    
    As discussed in SPARK-23285, this PR introduces a new configuation property `spark.kubernetes.executor.cores` for specifying the phyiscal CPU cores requested for each executor pod. This is to avoid changing the semantics of `spark.executor.cores` and `spark.task.cpus` and their role in task scheduling, task parallelism, dynamic resource allocation, etc. The new configuraiton property only determines the physical CPU cores available to an executor. An executor can still run multiple tasks simultaneously by using appropriate values for `spark.executor.cores` and `spark.task.cpus`.

----


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    What is the default value if it is not configured, how do K8S control the CPU usage by default?
    
    Also it seems that user may configure how to differentiate between k8s executor cores and executor cores.
    
     BTW, do we also need similar k8s driver cores for 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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178618810
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -549,14 +549,22 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
       </td>
     </tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.request.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). 
    +    This is distinct from <code>spark.executor.cores</code> and is only used for specifying the executor pod cpu request if set. Specifically, if this is set, its value is used to specify the executor 
    --- End diff --
    
    Done.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Agreed that `spark.kubernetes.executor.request.cores` is a better name. Will change it.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Any comment or concern on this? Is this good to merge?


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170138218
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodFactory.scala ---
    @@ -144,7 +149,7 @@ private[spark] class ExecutorPodFactory(
         val executorEnv = (Seq(
           (ENV_DRIVER_URL, driverUrl),
           // Executor backend expects integral value for executor cores, so round it up to an int.
    -      (ENV_EXECUTOR_CORES, math.ceil(executorCores).toInt.toString),
    +      (ENV_EXECUTOR_CORES, executorCores.toString),
    --- End diff --
    
    The comment above is now outdated and can be removed.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87626 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87626/testReport)** for PR 20553 at commit [`8f457ce`](https://github.com/apache/spark/commit/8f457cee17ffdb478fca3d3d2ff05c343217aef4).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1879/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87538/
    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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178605773
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -549,14 +549,23 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.request.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). 
    +    This is distinct from <code>spark.executor.cores</code> and is only used for specifying executor pod cpu request if set. Task parallelism, e.g., number of tasks an executor can
    +    run concurrently is not affected by this. 
    --- End diff --
    
    this should be clear when `spark.kubernetes.executor.request.cores` is set `spark.executor.cores` is ignore?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170137631
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) on the amount of CPU cores for the driver pod.
    --- End diff --
    
    I think it reads better without "the amount of", i.e. "Specify a hard limit on CPU cores for the driver pod". Same comment for the below section as well. 


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    The value of `spark.executor.cores` will be used to set cpu request for the executor pods if `spark.kubernetes.executor.cores` is not set. `spark.driver.cores` already allows fractional values so we don't have a Kubernetes specific property for the driver pod.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Merging once tests pass. Thanks!


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #88763 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88763/testReport)** for PR 20553 at commit [`cfe1eab`](https://github.com/apache/spark/commit/cfe1eab1bf8c282ce4c52d4c8ef9c5f3d3ac669a).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87626 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87626/testReport)** for PR 20553 at commit [`8f457ce`](https://github.com/apache/spark/commit/8f457cee17ffdb478fca3d3d2ff05c343217aef4).


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178371345
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). Takes precendence over <code>spark.executor.cores</code> if set.
    --- End diff --
    
    Precedence in what sense? This won't override `spark.executor.cores` when it comes to the number of concurrently running tasks. Think it's better to say that it's distinct from spark.executor.cores.


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178614696
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -549,14 +549,22 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
       </td>
     </tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.request.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). 
    +    This is distinct from <code>spark.executor.cores</code> and is only used for specifying the executor pod cpu request if set. Specifically, if this is set, its value is used to specify the executor 
    --- End diff --
    
    Might be good to also add typical ways (1.5m, 2.0, 5, etc) in which one can specify this and link to https://kubernetes.io/docs/tasks/configure-pod-container/assign-cpu-resource/#cpu-units.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/950/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/724/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    > This is to avoid changing the semantics of spark.executor.cores and spark.task.cpus and their role in task scheduling, task parallelism, dynamic resource allocation, etc. The new configuration property only determines the physical CPU cores available to an executor.
    
    Do you mean `spark.kubernetes.executor.cores` will only be used with k8s for static allocation? It looks to me that if we wanna k8s work with Spark dynamic allocation better, we have to change the semantics of `spark.executor.cores` to support fraction. Or we introduce a new dynamic allocation module for k8s, which reads `spark.kubernetes.executor.cores`.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    `spark.kubernetes.executor.cores` has nothing to do with dynamic resource allocation. It's just a way of letting users specify a value for the cpu resource request that conforms to Kubernetes convention and is only read/used when determining the cpu request. 


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1845/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87625 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87625/testReport)** for PR 20553 at commit [`11f94e2`](https://github.com/apache/spark/commit/11f94e2f57b6af331ec0d9e58708e03ac5a44e2a).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170138424
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) on the amount of CPU cores for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the amount of CPU cores to request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu).
    --- End diff --
    
    Should we mention that this value overrides `spark.executor.cores`?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1008/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1845/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1008/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    > Do you mean `spark.kubernetes.executor.cores` will only be used with k8s for static allocation? It looks to me that if we wanna k8s work with Spark dynamic allocation better, we have to change the semantics of `spark.executor.cores` to support fraction. Or we introduce a new dynamic allocation module for k8s, which reads `spark.kubernetes.executor.cores`.
    
    Since it's only used to define the **physical** cpu request for executor pods, it can be used for both static and dynamic allocation in k8s mode. IMO, `spark.executor.cores` and `spark.task.cpus` are for defining the cpu resource availability and demands in a virtual sense. An executor with `100m` physical cpus allocated can still run 2 tasks if they fit in. This is equivalently to a scenario in which `spark.executor.cores=2` and `spark.task.cpus=1`. Task parallelism per executor and dynamic resource allocation are still based on the semantics of `spark.executor.cores` and `spark.task.cpus`. So I don't think we need to change the semantics of these two properties and we don't need a new version of dynamic resource allocation for k8s. IMO, it's unfortunate that `spark.executor.cores` is used both for defining the physical cpu request and for defining task parallelism. 


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/950/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87234 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87234/testReport)** for PR 20553 at commit [`50ebb50`](https://github.com/apache/spark/commit/50ebb5068a35a9a0f2becd27153bdc7cc7aae251).


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87538 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87538/testReport)** for PR 20553 at commit [`8b711ce`](https://github.com/apache/spark/commit/8b711ce9d5a16992fbf1d4b3cb096f2d35f72559).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #88830 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88830/testReport)** for PR 20553 at commit [`a9db323`](https://github.com/apache/spark/commit/a9db32369bcac7e9316c9a37d98927ebb567c9d7).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178614227
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -549,14 +549,22 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
       </td>
     </tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.request.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). 
    +    This is distinct from <code>spark.executor.cores</code> and is only used for specifying the executor pod cpu request if set. Specifically, if this is set, its value is used to specify the executor 
    --- End diff --
    
    The two lines appear a bit redundant - can do with just one probably.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87234 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87234/testReport)** for PR 20553 at commit [`50ebb50`](https://github.com/apache/spark/commit/50ebb5068a35a9a0f2becd27153bdc7cc7aae251).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1879/



---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178371360
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala ---
    @@ -85,6 +85,12 @@ private[spark] object Config extends Logging {
           .stringConf
           .createOptional
     
    +  val KUBERNETES_EXECUTOR_CORES =
    +    ConfigBuilder("spark.kubernetes.executor.cores")
    +      .doc("Specify the cpu request for each executor pod")
    +      .stringConf
    +      .createOptional
    --- End diff --
    
    Can we use `fallbackConf` here?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    IIUC the `spark.kubernetes.executor.cores` here is just a special case for `spark.executor.cores`, for k8s backend, you shall still have to handle float values if you're to read the value of `spark.kubernetes.executor.cores`, for instance, in dynamic allocation. If that is the case here, I don't see much benefit of bringing in a new conf here.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Is this OK to merge?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178373167
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala ---
    @@ -85,6 +85,12 @@ private[spark] object Config extends Logging {
           .stringConf
           .createOptional
     
    +  val KUBERNETES_EXECUTOR_CORES =
    +    ConfigBuilder("spark.kubernetes.executor.cores")
    +      .doc("Specify the cpu request for each executor pod")
    +      .stringConf
    +      .createOptional
    --- End diff --
    
    Unfortunately, there's not a `ConfigEntry` for `spark.executor.cores` in core.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Ah never mind sorry - thought we were referring to changing thread count with `spark.kubernetes.executor.request.cores`. Think this config key makes sense to communicate the exact K8s semantics here.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87625 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87625/testReport)** for PR 20553 at commit [`11f94e2`](https://github.com/apache/spark/commit/11f94e2f57b6af331ec0d9e58708e03ac5a44e2a).


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170168795
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) on the amount of CPU cores for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the amount of CPU cores to request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu).
    --- End diff --
    
    Done.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    `spark.executor.cores` is the standard used by all the other cluster managers, so we have to use that.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1009/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    How do we plan to support dynamic allocation with k8s? Should we read `spark.executor.cores` or `spark.kubernetes.executor.cores` ?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/724/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Oops - knew we tried that before. Just paged in the discussion from https://github.com/apache/spark/pull/20460. Looks like that approach might need revisiting. 


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r168943784
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala ---
    @@ -85,6 +85,12 @@ private[spark] object Config extends Logging {
           .stringConf
           .createOptional
     
    +  val KUBERNETES_EXECUTOR_CORES =
    +    ConfigBuilder("spark.kubernetes.executor.cores")
    +      .doc("Specify the CPU core request for each executor pod")
    --- End diff --
    
    nit: it was lower case "cpu" in other config, and no "core"


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1880/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    @cloud-fan @jiangxb1987 I assume you're referring to [ExecutorAllocationManager.scala#L114-L118](https://github.com/apache/spark/blob/fc6fe8a1d0f161c4788f3db94de49a8669ba3bcc/core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala#L114-L118)
    
    @liyinan926, while this might be the least obtrusive way to get Kubernetes mode to have fractional executor cores and not change other backends, it seems to me like separating the notion of a pod's CPU request and `spark.executor.cores` might prove confusing to users. We don't expect Spark users to necessarily understand pods or containers. Elaborating on @srowen's point on the JIRA, in theory, could we relax `spark.executor.cores` to accept doubles and then push the check down to the individual resource managers - force conversion to int if needed? It might require a similar change to `spark.task.cpus` also possibly. Thoughts?


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    also cc @cloud-fan @jerryshao 


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1880/



---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170168480
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodFactory.scala ---
    @@ -144,7 +149,7 @@ private[spark] class ExecutorPodFactory(
         val executorEnv = (Seq(
           (ENV_DRIVER_URL, driverUrl),
           // Executor backend expects integral value for executor cores, so round it up to an int.
    -      (ENV_EXECUTOR_CORES, math.ceil(executorCores).toInt.toString),
    +      (ENV_EXECUTOR_CORES, executorCores.toString),
    --- End diff --
    
    Done.


---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178373194
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). Takes precendence over <code>spark.executor.cores</code> if set.
    --- End diff --
    
    Fixed.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #87538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87538/testReport)** for PR 20553 at commit [`8b711ce`](https://github.com/apache/spark/commit/8b711ce9d5a16992fbf1d4b3cb096f2d35f72559).


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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/956/
    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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r168972337
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala ---
    @@ -85,6 +85,12 @@ private[spark] object Config extends Logging {
           .stringConf
           .createOptional
     
    +  val KUBERNETES_EXECUTOR_CORES =
    +    ConfigBuilder("spark.kubernetes.executor.cores")
    +      .doc("Specify the CPU core request for each executor pod")
    --- End diff --
    
    Done.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Can we call this `spark.kubernetes.executor.request.cores`, similar to `spark.kubernetes.executor.limit.cores` ?
    I think this would make the naming a bit less confusing.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    **[Test build #88826 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88826/testReport)** for PR 20553 at commit [`f103413`](https://github.com/apache/spark/commit/f10341359dcfd2aaf2cf3c1c6cf19c47f1e352f8).
     * 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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r178612630
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -549,14 +549,23 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard cpu [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +  </td>
    +</tr>
    +<tr>
    +  <td><code>spark.kubernetes.executor.request.cores</code></td>
    +  <td>(none)</td>
    +  <td>
    +    Specify the cpu request for each executor pod. Values conform to the Kubernetes [convention](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-cpu). 
    +    This is distinct from <code>spark.executor.cores</code> and is only used for specifying executor pod cpu request if set. Task parallelism, e.g., number of tasks an executor can
    +    run concurrently is not affected by this. 
    --- End diff --
    
    Done.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/1009/



---

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


[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...

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

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


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    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 #20553: [SPARK-23285][K8S] Add a config property for spec...

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

    https://github.com/apache/spark/pull/20553#discussion_r170169357
  
    --- Diff: docs/running-on-kubernetes.md ---
    @@ -576,14 +576,21 @@ specific to Spark on Kubernetes.
       <td><code>spark.kubernetes.driver.limit.cores</code></td>
       <td>(none)</td>
       <td>
    -    Specify the hard CPU [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for the driver pod.
    +    Specify a hard [limit](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) on the amount of CPU cores for the driver pod.
    --- End diff --
    
    Done.


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

    https://github.com/apache/spark/pull/20553
  
    @foxish I think the confusion mostly comes from the property name. I don't think we need to change the semantics of `spark.executor.cores` nor the way dynamic resource allocation works in K8S mode. Like what I said above, `spark.executor.cores`/`spark.task.cpus` still determines the number of tasks that can run in an executor simultaneously given the cpus specified in `spark.kubernetes.executor.cores`.   


---

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


[GitHub] spark issue #20553: [SPARK-23285][K8S] Add a config property for specifying ...

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

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


---

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