You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/02/24 00:46:25 UTC
[GitHub] [spark] Yikun opened a new pull request #35640: [WIP][SPARK-38187][K8S] Support resource reservation with volcano implementations
Yikun opened a new pull request #35640:
URL: https://github.com/apache/spark/pull/35640
### What changes were proposed in this pull request?
- Add two configurations: and `spark.kubernetes.job.minCPU` and `spark.kubernetes.job.minMemory`
- Apply min resource info into volcano podgroup
### Why are the changes needed?
Support resource reservation with volcano implementations
### Does this PR introduce _any_ user-facing change?
Yes, configuration introduced
### How was this patch tested?
- UT
- integration test to be done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820525316
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
Yes, this might be a good suggestion for improvement. I also searched in spark, only `spark.task.cpus` are using `cpu`, and others are using `cores`. I guess it might be because of the name originally inherited from Yarn.
So, I think `cores` is also fine for me. if no objection I can change this soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #35640: [SPARK-38187][K8S][TESTS] Add K8S IT for `volcano` minResources cpu/memory spec
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35640:
URL: https://github.com/apache/spark/pull/35640
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [WIP][SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820458200
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,7 +196,58 @@ private[spark] trait VolcanoTestsSuite extends BeforeAndAfterEach { k8sSuite: Ku
)
}
- test("SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled)", k8sTestTag, volcanoTag) {
+ private def verifyJobsSucceededOneByOne(jobNum: Int, groupName: String): Unit = {
+ // Check Pending jobs completed one by one
+ (1 until jobNum).map { completedNum =>
+ Eventually.eventually(TIMEOUT, INTERVAL) {
+ val pendingPods = getPods(role = "driver", groupName, statusPhase = "Pending")
+ assert(pendingPods.size === jobNum - completedNum)
+ }
+ }
+ // All jobs succeeded finally
+ Eventually.eventually(TIMEOUT, INTERVAL) {
+ val succeededPods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+ assert(succeededPods.size === jobNum)
+ }
+ }
+
+ test("SPARK-38187: Run SparkPi Jobs with minCPU", k8sTestTag, volcanoTag) {
+ val groupName = generateGroupName("min-cpu")
+ // Create a queue with 2 CPU, 3G memory capacity
+ createOrReplaceYAMLResource(QUEUE_2U_3G_YAML)
+ // Submit 3 jobs with minCPU = 2
+ val jobNum = 3
+ (1 to jobNum).map { i =>
+ Future {
+ runJobAndVerify(
+ i.toString,
+ groupLoc = Option(groupName),
+ queue = Option("queue-2u-3g"),
+ minCPU = Option("2"))
+ }
+ }
+ verifyJobsSucceededOneByOne(jobNum, groupName)
+ }
+
+ test("SPARK-38187: Run SparkPi Jobs with minMemory", k8sTestTag, volcanoTag) {
+ val groupName = generateGroupName("min-mem")
+ // Create a queue with 2 CPU, 3G memory capacity
+ createOrReplaceYAMLResource(QUEUE_2U_3G_YAML)
+ // Submit 3 jobs with minMemory = 3g
+ val jobNum = 3
+ (1 to jobNum).map { i =>
+ Future {
+ runJobAndVerify(
+ i.toString,
+ queue = Option("queue-2u-3g"),
+ groupLoc = Option(groupName),
+ minMemory = Option("3g"))
+ }
+ }
+ verifyJobsSucceededOneByOne(jobNum, groupName)
+ }
+
+ test("SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable)", k8sTestTag, volcanoTag) {
Review comment:
This looks like reverting to the original typo, `enable`. Could you recheck this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused.
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
If we are in the same page then:
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [kubernetesClient.resourceList(preKubernetesResources: _*).createOrReplace()]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only return volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819790005
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
Got it. Thanks, @Yikun .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused:
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
If we are in the same page then:
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [kubernetesClient.resourceList(preKubernetesResources: _*).createOrReplace()]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only return volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed resource amounts, this also helps to enhance the utilization of cluster in some level when cluster resource is limited**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1059908656
```
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (12 seconds, 211 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (34 seconds, 483 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (34 seconds, 345 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable) (17 seconds, 298 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enable) (37 seconds, 704 milliseconds)
[info] Run completed in 2 minutes, 18 seconds.
[info] Total number of tests run: 5
[info] Suites: completed 2, aborted 0
[info] Tests: succeeded 5, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [WIP][SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814286423
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
I'm just wondering if we can reuse some of the Spark configuration.
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
+ <td><code>(none)</code></td>
+ <td>
+ The minimum CPU for running the job. This info
+ will be stored in configuration and passed to specified feature step.
+ (such as `VolcanoFeatureStep`).
+ </td>
+ <td>3.3.0</td>
+</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minMemory</code></td>
Review comment:
Ditto.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1060729670
```
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (13 seconds, 248 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (33 seconds, 501 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (34 seconds, 336 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled) (17 seconds, 203 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enabled) (29 seconds, 392 milliseconds)
[info] - SPARK-38423: Run SparkPi Jobs with priorityClassName (21 seconds, 190 milliseconds)
[info] - SPARK-38423: Run driver job to validate priority order (20 seconds, 239 milliseconds)
[info] Run completed in 2 minutes, 54 seconds.
[info] Total number of tests run: 7
[info] Suites: completed 2, aborted 0
[info] Tests: succeeded 7, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 362 s (06:02), completed Mar 7, 2022 10:07:21 PM
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. Especially, when users don't want to set `minRes` strictly to which the spark job real needed.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061266014
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation.（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml? and do format in feature step? This perhaps not flexiable, and a little bit hard to maintain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1063554636
@dongjoon-hyun Done! Thanks! : )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820454157
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
We need to document what is used in case of `(none)`, @Yikun .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed resource mount**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820197483
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
also cc @holdenk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1060272040
@dongjoon-hyun Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation.（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819997509
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
@dongjoon-hyun addressed, will do a rebase after it approved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819997509
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
@dongjoon-hyun addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820456358
##########
File path: resource-managers/kubernetes/integration-tests/src/test/resources/volcano/queue-2u-3g.yml
##########
@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+apiVersion: scheduling.volcano.sh/v1beta1
+kind: Queue
+metadata:
+ name: queue-2u-3g
+spec:
+ weight: 1
+ capability:
+ cpu: "2"
+ memory: "3072Mi"
Review comment:
Shall we use `3Gi` simply?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820455668
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -47,9 +51,25 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
.withName(podGroupName)
.withNamespace(namespace)
.endMetadata()
+ .editOrNewSpec()
+ .withMinMember(MIN_MEMBER)
+ .endSpec()
queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ minCPU.foreach{ cpu =>
+ val cpuQ = new QuantityBuilder(false)
+ .withAmount(s"${cpu}")
Review comment:
It seems that we don't need `{` and `}` .
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -47,9 +51,25 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
.withName(podGroupName)
.withNamespace(namespace)
.endMetadata()
+ .editOrNewSpec()
+ .withMinMember(MIN_MEMBER)
+ .endSpec()
queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ minCPU.foreach{ cpu =>
+ val cpuQ = new QuantityBuilder(false)
+ .withAmount(s"${cpu}")
+ .build()
+ podGroup.editOrNewSpec().addToMinResources("cpu", cpuQ).endSpec()
+ }
+ minMemory.foreach{ memory =>
+ val memoryQ = new QuantityBuilder(false)
+ .withAmount(s"${memory}")
Review comment:
ditto.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820459455
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
BTW, this naming looks inconsistent to me. For the other K8s configuration, we use `Core` instead of `CPU`. Please double check this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820545110
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
```
If no value is specified, there will be no limits on the minimum resources.
```
will add it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820545110
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
```
If no value is specified, there will be no lower limit on job-level CPU resources.
```
will add it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed resource amounts**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1060192732
https://github.com/apache/spark/pull/35733 is merged now . Could you rebase this PR, @Yikun ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819794771
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
I added my comment about `stackable` clean-up there.
- https://github.com/apache/spark/pull/35733#pullrequestreview-900547357
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35640: [WIP][SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1058422005
Please check the dev mailing list . Ramping down for Apache Spark 3.3 release started, @Yikun .
- https://lists.apache.org/thread/ffdk52hkvgsc5ncjh1z0nv120jowtrld
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S][TESTS] Add K8S IT for resource reservation (Spark K8S with volcano)
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1063558675
```
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (12 seconds, 738 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled) (13 seconds, 294 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enabled) (25 seconds, 659 milliseconds)
[info] - SPARK-38423: Run SparkPi Jobs with priorityClassName (19 seconds, 310 milliseconds)
[info] - SPARK-38423: Run driver job to validate priority order (16 seconds, 467 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (29 seconds, 546 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (30 seconds, 473 milliseconds)
[info] Run completed in 2 minutes, 30 seconds.
[info] Total number of tests run: 7
[info] Suites: completed 2, aborted 0
[info] Tests: succeeded 7, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 236 s (03:56), completed 2022-3-10 9:17:46
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused.
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed resource amounts, this also helps to enhance the utilization of cluster in some level when cluster resource is limited**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
So, we might add the basic minRes configuration first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820525316
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
Yes, this might be a good suggestion for improvement. I also searched in spark, only `spark.task.cpus` are using `cpu`, and others are using `cores`. I guess it (`cores`) might be because of the name originally inherited from Yarn.
So, I think `cores` is also fine for me, like `spark.kubernetes.job.minCores`. if no objection I can change this soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1063536283
Could you rebase this PR to the master and convert it as a test case addition PR, @Yikun ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1059267152
```
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (12 seconds, 199 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (45 seconds, 453 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (43 seconds, 329 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable) (17 seconds, 385 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enable) (37 seconds, 684 milliseconds)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] martin-g commented on a change in pull request #35640: [WIP][SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r817686949
##########
File path: resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStepSuite.scala
##########
@@ -48,6 +48,18 @@ class VolcanoFeatureStepSuite extends SparkFunSuite {
assert(podGroup.getSpec.getQueue === "queue1")
}
+ test("SPARK-38187: Support `spark.kubernetes.job.minCPU/minMemory`") {
Review comment:
`volcanoTag` would be useful!
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -151,6 +157,54 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
)
}
+ test("SPARK-38187: Run SparkPi Jobs with minCPU", k8sTestTag, volcanoTag) {
+ // Create a queue with 2 CPU, 3G memory capacity
+ createOrReplaceYAMLResource(VOLCANO_RES_LIMIT_QUEUE_YAML)
+ val jobNum = 4
+ (1 to jobNum).map { i =>
+ Future {
+ runJobAndVerify(
+ i.toString,
+ groupLoc = Option(GROUP_PREFIX),
+ queue = Option("queue-limit-resource"),
+ minCPU = Option("2"))
+ }
+ }
+ Eventually.eventually(TIMEOUT, INTERVAL) {
+ // Only 1 job running or all Succeeded
+ val pendingPods = getPods(role = "driver", GROUP_PREFIX, statusPhase = "Running")
+ assert(pendingPods.size <= 1)
+ // All jobs succeeded finally
+ val succeededPods = getPods(role = "driver", GROUP_PREFIX, statusPhase = "Succeeded")
+ assert(succeededPods.size === jobNum)
+ }
+ deleteYAMLResource(VOLCANO_RES_LIMIT_QUEUE_YAML)
Review comment:
The deletion of the resources should be in `finally` or `after()` method.
Currently if a test fails the resources are not removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820545110
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
```
If it is `(none)`, indicates there are no limits on the minimum resources.
```
will add it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r814613104
##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,26 @@ See the [configuration page](configuration.html) for information on Spark config
</td>
<td>3.3.0</td>
</tr>
+<tr>
+ <td><code>spark.kubernetes.job.minCPU</code></td>
Review comment:
Yep, this's a good question, there were some related use case of Spark with volcano in production can be shared in here:
- Case1 (this PR): `minCPU` = specified `minCPU`: for users who know their own cluster resources well, this is the basic user case. **Especially, when users don't want to set `minRes` strictly to which the spark job real needed resource amounts, this also enhance the utilization of cluster in some level when cluster resource is limited**.
- Case2: `minCPU` = driver.request + `executor.number` * `executor.request`: for users who don't care much about job resource usage.
- Case3: `minCPU` = (driver.request + `executor.number` * `executor.request`) * `factor`, for users want to guarantee the resources of the job in some level, but also want to improve the utilization of the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused:
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
If we are in the same page then:
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [kubernetesClient.resourceList(preKubernetesResources: _*).createOrReplace()]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only return volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused:
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are **only one podgroup is created for one job** in current implementation（the pre kubernetes only calls in driver side).
If we are in the same page then:
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [kubernetesClient.resourceList(preKubernetesResources: _*).createOrReplace()]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only return volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820525316
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
Yes, this might be a good suggestion for improvement. I also searched in spark, only `spark.task.cpus` are using `cpu`, and others are using `cores`. I guess it might be because of the name originally inherited from Yarn.
So, I think `cores` is also fine for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r820525316
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
Yes, this might be a good suggestion for improvement. I also searched in spark, only `spark.task.cpus` are using `cpu`, and others are using `cores`. I guess it (`cores`) might be because of the name originally inherited from Yarn.
So, I think `cores` is also fine for me. if no objection I can change this soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused.
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819695448
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
FYI, These code are also submited into a separate PR: https://github.com/apache/spark/pull/35733
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1060271818
```
$ build/sbt -Pvolcano -Pkubernetes -Pkubernetes-integration-tests -Dtest.exclude.tags=minikube,r -Dtest.include.tags=volcano -Dspark.kubernetes.test.namespace=default "kubernetes-integration-tests/test"
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (12 seconds, 245 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (43 seconds, 473 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (43 seconds, 354 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable) (18 seconds, 323 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enable) (38 seconds, 673 milliseconds)
[info] Run completed in 2 minutes, 38 seconds.
[info] Total number of tests run: 5
[info] Suites: completed 2, aborted 0
[info] Tests: succeeded 5, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 378 s (06:18), completed Mar 7, 2022 3:28:39 PM
$ k get pod
No resources found in default namespace.
$ k get queue
NAME AGE
default 2d20h
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r819695448
##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoSuite.scala
##########
@@ -25,6 +30,25 @@ class VolcanoSuite extends KubernetesSuite with VolcanoTestsSuite {
sparkAppConf
.set("spark.kubernetes.driver.scheduler.name", "volcano")
.set("spark.kubernetes.executor.scheduler.name", "volcano")
+ testGroups = mutable.Set.empty
+ }
+
+ private def deletePodInTestGroup(): Unit = {
Review comment:
FYI, These code also submited into a separate PR: https://github.com/apache/spark/pull/35733
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on a change in pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35640:
URL: https://github.com/apache/spark/pull/35640#discussion_r821295005
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -299,6 +300,20 @@ private[spark] object Config extends Logging {
.stringConf
.createOptional
+ val KUBERNETES_JOB_MIN_CPU = ConfigBuilder("spark.kubernetes.job.minCPU")
Review comment:
also cc @holdenk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation.（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml? and do format in feature step? This perhaps not flexiable, and a little bit hard to maintain.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand if something confused.
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once before driver creation. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
If we are in the same page then:
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [kubernetesClient.resourceList(preKubernetesResources: _*).createOrReplace()]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only return volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061267316
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod (driver and executors) one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml and do format in feature step. This perhaps not flexiable, and a little bit hard to maintain. and also introduce a volcano client module deps.
:), feel free to left any question you had, I will try my best to discussion to make work better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun removed a comment on pull request #35640: [SPARK-38187][K8S] Support resource reservation with volcano implementations
Posted by GitBox <gi...@apache.org>.
Yikun removed a comment on pull request #35640:
URL: https://github.com/apache/spark/pull/35640#issuecomment-1061266014
@dongjoon-hyun I'm not sure there were some misunderstanding, something below might help you understand.
We configure pod one by one but we only return the PreAdditionalK8SResource once. So there are only one podgroup is created for one job in current implementation.（the pre kubernetes only calls in driver side).
> It seems that you decided not to use volcano API for some reasons
No more hidden reason, just make sure it can be more generic, then it can be created by [prekubernetes create]( https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala#L140-L140). So we only use volcano model in here.
> Can we use new DefaultVolcanoClient().podGroups().load()?
if you meaned use this to build volcano-model, client side yaml load could be a alternative way to complete this, but I think we still need to left placeholder in yaml? and do format in feature step? This perhaps not flexiable, and a little bit hard to maintain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org