[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.cores + (none) + +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 spark.executor.cores if set. --- End diff -- Fixed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.request.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.request.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + spark.kubernetes.executor.request.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + spark.kubernetes.executor.request.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + spark.kubernetes.executor.request.cores + (none) + +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 spark.executor.cores 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/20553 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[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 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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.cores + (none) + +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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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. + + + + spark.kubernetes.executor.cores + (none) + +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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20553: [SPARK-23285][K8S] Add a config property for spec...
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. spark.kubernetes.driver.limit.cores (none) -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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org