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/03/08 22:04:23 UTC
[GitHub] [spark] dongjoon-hyun opened a new pull request #35776: [SPARK-38455][K8S] Support driver/executor PodGroup templates
dongjoon-hyun opened a new pull request #35776:
URL: https://github.com/apache/spark/pull/35776
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
--
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] viirya commented on a change in pull request #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822147629
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
Review comment:
Got it. 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] Yikun commented on a change in pull request #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822247430
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I have only one left conern in priorityClassName about configuration overwrite priority.
Compare to pod template this might be a reasonable overwirte sort for me:
- Top 1 configuration value (like queue)
- Top 2 config template value (like driver/executor template)
- Top 3 default value/feature (like priorityClassName which reuse pod priority as default value)
So do you think we should change it to:
```
// Overwrite if queue configuration specified
queue.foreach(spec.setQueue(_))
// Reuse Pod priority if priority is not set in template
if (spec.getPriorityClassName == null) priorityClassName.foreach(spec.setPriorityClassName(_))
```
Then users can set `PriorityClassName` by specifying priority in template. WDYT?
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822247430
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I have only one left conern in priorityClassName about configuration overwrite priority.
Compare to pod template this might be a reasonable overwirte sort for me:
- Top 1 configuration value (like queue)
- Top 2 config template value (like driver/executor template)
- Top 3 default value/feature (like priorityClassName which reuse pod priority as default value)
So do you think we should change it to:
```
// Overwrite if queue configuration specified
queue.foreach(spec.setQueue(_))
// Reuse Pod priority if priority is not set in template
if (spec.getPriorityClassName == null) priorityClassName.foreach(spec.setPriorityClassName(_))
```
Then users can set `PriorityClassName` by specify priority in template
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822261650
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I understand you are exploring all possibility. However, Apache Spark doesn't work like that. :) We prefer the user given configurations over any template files and it's simpler always. So, the current behavior is intentionally consistent with the existing one.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822143931
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
Review comment:
Yes, currently, `spark.kubernetes.job.queue` configuration will overwrite it. It's the behavior for non-Volcano driver/executor pod template, too.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062333903
I take a roughly look it's a better choice to help to avoid configuration introduced. I will take a deep look and test this today.
@dongjoon-hyun 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] Yikun commented on a change in pull request #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822151624
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
Review comment:
Note that we are not support executor's separated podgroup yet, currently no ability to create pre resource for excutors in spark on k8s.
we only supported driver side podgroup or job level podgroup (share driver podgroup) now.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062538577
> We can take advantage this PodGroup templates more actively in order to isolate from the other customer schedulers, the default K8s scheduler or Apache Spark itself. I'm going to propose that after this PR.
Expected!
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062277884
Could you review this, @viirya and @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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822234762
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
Review comment:
Ya, this is added for feature parity for your existing contribution.
`getAdditionalPreKubernetesResources` is used only for `KubernetesDriverBuilder`.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822247430
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I have only one left conern in priorityClassName about configuration overwrite priority.
Compare to pod template this might be a reasonable overwirte sort for me:
- Top 1 configuration value (like queue)
- Top 2 config template value (like driver/executor template)
- Top 3 default value (like priorityClassName which reuse pod priority as default value)
So do you think we should change it to:
```
// Overwrite if queue configuration specified
queue.foreach(spec.setQueue(_))
// Reuse Pod priority if priority is not set in template
if (spec.getPriorityClassName == null) priorityClassName.foreach(spec.setPriorityClassName(_))
```
Then users can set `PriorityClassName` by specifying priority in template. WDYT?
--
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] viirya commented on a change in pull request #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822149156
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
Review comment:
If no template given, do we need some default pod group spec?
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822263156
##########
File path: resource-managers/kubernetes/core/src/test/resources/executor-podgroup-template.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: PodGroup
+spec:
+ minMember: 1000
+ minResources:
+ cpu: "4"
+ memory: "16Gi"
+ priorityClassName: executor-priority
+ queue: executor-queue
Review comment:
Fixed now, @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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822271553
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
OK, I can understand it. This is not only exploring all possibility, but also there are many users want to specify priority more flexiable in job level.
An alternative way to help user use priority conveniently, we still keep this as your current implemtation, consider priority scheduling is very common case, do you think we could introduce a configuration like `spark.kuberentes.driver.PriorityClassName` to simplify config template? This also help both **native default scheduler** and also **custom scheduler** to specify spark job priority easily.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062537736
Thank you, @viirya and @Yikun . Merged to master for Apache Spark 3.3.0.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062335290
Thanks, @viirya and @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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822235094
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
Review comment:
Yes, it's the existing code's behavior at `PodGroupBuilder`.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822247430
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I have only one left conern in priorityClassName about configuration overwrite priority.
Compare to pod template this might be a reasonable overwirte sort for me:
- Top 1 configuration value (like queue)
- Top 2 config template value (like driver/executor template)
- Top 3 default value/feature (like priorityClassName which reuse pod priority as default value)
So do you think we should change it to:
```
// Overwrite if queue configuration specified
queue.foreach(spec.setQueue(_))
// Reuse Pod priority if priority is not set in template
if (spec.getPriorityClassName == null) priorityClassName.foreach(spec.setPriorityClassName(_))
```
Then users can set `PriorityClassName` by specifying priority in template
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822248403
##########
File path: resource-managers/kubernetes/core/src/test/resources/executor-podgroup-template.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: PodGroup
+spec:
+ minMember: 1000
+ minResources:
+ cpu: "4"
+ memory: "16Gi"
+ priorityClassName: executor-priority
+ queue: executor-queue
Review comment:
nit: new line
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
+ priorityClassName.foreach(spec.setPriorityClassName(_))
Review comment:
I have only one left conern in priorityClassName about configuration overwrite priority.
Compare to pod template this might be a reasonable overwirte sort for me:
- Top 1 configuration value (like queue)
- Top 2 config template value (like driver/executor template)
- Top 3 default value/feature (like priorityClassName which reuse pod priority as default value)
So do you think we should change it to:
```
// Overwrite if queue configuration specified
queue.foreach(spec.setQueue(_))
// Reuse Pod priority if priority is not set in template
if (spec.getPriorityClassName == null) priorityClassName.foreach(spec.setPriorityClassName(_))
```
Then we can use config template to overwrite priority the default feature step value if user needed.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062497415
To fix license linter error, the licenses are added.
BTW, do you have any other concerns, @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] viirya commented on a change in pull request #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822138229
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
+ }
+ val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
+ var metadata = pg.getMetadata
+ if (metadata == null) metadata = new ObjectMeta
+ metadata.setName(podGroupName)
+ metadata.setNamespace(namespace)
+ pg.setMetadata(metadata)
- priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())
+ var spec = pg.getSpec
+ if (spec == null) spec = new PodGroupSpec
+ queue.foreach(spec.setQueue(_))
Review comment:
I saw there is queue in the PodGroup template. So this will overwrite 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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822151624
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -43,19 +44,27 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
}
override def getAdditionalPreKubernetesResources(): Seq[HasMetadata] = {
- val podGroup = new PodGroupBuilder()
- .editOrNewMetadata()
- .withName(podGroupName)
- .withNamespace(namespace)
- .endMetadata()
- .editOrNewSpec()
- .endSpec()
+ val client = new DefaultVolcanoClient
- queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
+ val template = if (kubernetesConf.isInstanceOf[KubernetesDriverConf]) {
+ kubernetesConf.get(KUBERNETES_DRIVER_PODGROUP_TEMPLATE_FILE)
+ } else {
+ kubernetesConf.get(KUBERNETES_EXECUTOR_PODGROUP_TEMPLATE_FILE)
Review comment:
Note that we are not support executor's separated podgroup yet, current no ability to create pre resource for excutors in spark on k8s.
we only supported driver side podgroup or job level podgroup (share driver podgroup) now.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35776:
URL: https://github.com/apache/spark/pull/35776#discussion_r822261976
##########
File path: resource-managers/kubernetes/core/src/test/resources/executor-podgroup-template.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: PodGroup
+spec:
+ minMember: 1000
+ minResources:
+ cpu: "4"
+ memory: "16Gi"
+ priorityClassName: executor-priority
+ queue: executor-queue
Review comment:
Sure.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35776:
URL: https://github.com/apache/spark/pull/35776#issuecomment-1062534414
BTW, @Yikun . We can take advantage this `PodGroup` templates more actively in order to isolate from the other customer schedulers, the default K8s scheduler or Apache Spark itself. I'm going to propose that after this PR.
--
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 #35776: [SPARK-38455][SPARK-38187][K8S] Support driver/executor `PodGroup` templates
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35776:
URL: https://github.com/apache/spark/pull/35776
--
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