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:35:17 UTC

[GitHub] [spark] Yikun opened a new pull request #35639: [WIP][SPARK-38188][K8S] Support priority scheduling with Volcano implementations

Yikun opened a new pull request #35639:
URL: https://github.com/apache/spark/pull/35639


   ### What changes were proposed in this pull request?
   Add a new configuration: `spark.kubernetes.job.priorityClassName`
   
   
   ### Why are the changes needed?
   Support priority scheduling with Volcano implementations
   
   ### Does this PR introduce _any_ user-facing change?
   Yes, configuration introduced
   
   
   ### How was this patch tested?
   - UT
   - the 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 pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057715536


   @dongjoon-hyun Thanks, your PR can make sure priority class name work basiclly.
   
    I also drafted a[ multi-job IT](https://github.com/Yikun/spark/commit/675f56b8ba97c22c1d572e6b07501c5af3b8607e#diff-e8b4a160e455ae14cdc70e658c40e870c29a880f40681f1bb463a61efdb5f242R230) before. Do you think we need this? TLDR: First, use a pod to stuck all jobs, then to check all jobs scheduled time order by priority value.


-- 
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 #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35639:
URL: https://github.com/apache/spark/pull/35639


   


-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1055849869


   > If we don't merge this (which I think yeah pod template is a reasonable alternative), let's document how to set the priority class with the pod template.
   
   +1 for @holdenk 's suggestion. 


-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180270



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       After this merged: https://github.com/apache/spark/pull/35733, it would be if you can take a look on this. :)
   
   We can cleanup the end deletions. I will add more notes.




-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180270



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       after this merged: https://github.com/apache/spark/pull/35733
   
   We can cleanup the end deletions. I will add more notes.




-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820177616



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       What are the two "TODO" for? Do you need to add other JIRA for 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] dongjoon-hyun commented on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057231710


   You can reuse SPARK-38189 by converting into a documentation 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 a change in pull request #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180193



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -201,4 +319,21 @@ private[spark] object VolcanoTestsSuite extends SparkFunSuite {
     getClass.getResource("/volcano/disable-queue0-enable-queue1.yml").getFile
   ).getAbsolutePath
   val GROUP_PREFIX = "volcano-test" + UUID.randomUUID().toString.replaceAll("-", "")
+  val VOLCANO_PRIORITY_YAML
+    = new File(getClass.getResource("/volcano/priorityClasses.yml").getFile).getAbsolutePath
+  val HIGH_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/high-priority-driver-template.yml").getFile
+  ).getAbsolutePath
+  val MEDIUM_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/medium-priority-driver-template.yml").getFile
+  ).getAbsolutePath
+  val LOW_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/low-priority-driver-template.yml").getFile
+  ).getAbsolutePath

Review comment:
       ah, this can be cleanup now, will address




-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1058801691


   Replace by https://github.com/apache/spark/pull/35728


-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057719475


   +1 for adding those tests for `Volcano` module, @Yikun .
   > // Prepare the priority resource
   >  createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)


-- 
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 #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1060256680


   ```
   [info] VolcanoSuite:
   [info] - Run SparkPi with volcano scheduler (13 seconds, 276 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled) (18 seconds, 289 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enabled) (27 seconds, 377 milliseconds)
   [info] - SPARK-38423: Run SparkPi Jobs with priorityClassName (21 seconds, 209 milliseconds)
   [info] - SPARK-38423: Run driver job to validate priority order (19 seconds, 212 milliseconds)
   [info] Run completed in 1 minute, 43 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: 321 s (05:21), completed Mar 7, 2022 3:04:23 PM
   ```
   Especially, check the rebase diff work.
   ```
   $ 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"
   
   $ k get pod
   No resources found in default namespace.
   
   $ k get queue
   NAME      AGE
   default   2d19h
   ```


-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059543778


   Well, I don't think #35728 replaces this. You need write a document for `Volcano` pod scheduling with volcano priority.


-- 
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 edited a comment on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057711510


   To help you make your Volcano Job priority PR, I made a `Pod`-spec `priorityClassName` test case PR independently.
   - https://github.com/apache/spark/pull/35716


-- 
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 closed pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun closed pull request #35639:
URL: https://github.com/apache/spark/pull/35639


   


-- 
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 #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1060192384


   #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] Yikun edited a comment on pull request #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1060256680


   ```
   [info] VolcanoSuite:
   [info] - Run SparkPi with volcano scheduler (13 seconds, 276 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled) (18 seconds, 289 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enabled) (27 seconds, 377 milliseconds)
   [info] - SPARK-38423: Run SparkPi Jobs with priorityClassName (21 seconds, 209 milliseconds)
   [info] - SPARK-38423: Run driver job to validate priority order (19 seconds, 212 milliseconds)
   [info] Run completed in 1 minute, 43 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: 321 s (05:21), completed Mar 7, 2022 3:04:23 PM
   ```
   Especially, check the rebase diff work.
   ```
   $ 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"
   
   $ k get pod
   No resources found in default namespace.
   
   $ k get queue
   NAME      AGE
   default   2d19h
   
   k get priorityclass
   NAME                      VALUE        GLOBAL-DEFAULT   AGE
   system-cluster-critical   2000000000   false            24d
   system-node-critical      2000001000   false            24d
   ```


-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057715536


   @dongjoon-hyun Thanks, your PR can make sure priority class name work basiclly.
   
    I also drafted a[ multi-job IT](https://github.com/Yikun/spark/commit/675f56b8ba97c22c1d572e6b07501c5af3b8607e#diff-e8b4a160e455ae14cdc70e658c40e870c29a880f40681f1bb463a61efdb5f242R230) before. Do you think we need this? TLDR: First, use a pod to stuck all jobs, then to check all jobs scheduled time order by priority value.
   
   Of course, I still need to do a further simplify such as use system priority directly


-- 
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] holdenk commented on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1055847723


   If we don't merge this (which I think yeah pod template is a reasonable alternative), let's document how to set the priority class with the pod 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 pull request #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059914689


   ```
   [info] VolcanoSuite:
   [info] - Run SparkPi with volcano scheduler (12 seconds, 238 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable) (18 seconds, 354 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enable) (38 seconds, 685 milliseconds)
   [info] - SPARK-38423: Run SparkPi Jobs with priorityClassName (21 seconds, 293 milliseconds)
   [info] - SPARK-38423: Run driver job to validate priority order (19 seconds, 304 milliseconds)
   [info] Run completed in 1 minute, 53 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: 152 s (02:32), completed Mar 6, 2022 3:58:34 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] dongjoon-hyun commented on pull request #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1060286481


   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] Yikun commented on pull request #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059890214


   ```
   [info] VolcanoSuite:
   [info] - Run SparkPi with no resources (13 seconds, 181 milliseconds)
   [info] - Run SparkPi with no resources & statefulset allocation (12 seconds, 772 milliseconds)
   [info] - Run SparkPi with a very long application name. (11 seconds, 736 milliseconds)
   [info] - Use SparkLauncher.NO_RESOURCE (12 seconds, 821 milliseconds)
   [info] - Run SparkPi with a master URL without a scheme. (11 seconds, 743 milliseconds)
   [info] - Run SparkPi with an argument. (13 seconds, 714 milliseconds)
   [info] - Run SparkPi with custom labels, annotations, and environment variables. (11 seconds, 781 milliseconds)                            
   [info] - All pods have the same service account by default (11 seconds, 737 milliseconds)
   [info] - Run extraJVMOptions check on driver (5 seconds, 702 milliseconds)
   [info] - Run SparkRemoteFileTest using a remote data file (12 seconds, 912 milliseconds)
   [info] - Verify logging configuration is picked from the provided SPARK_CONF_DIR/log4j2.properties (20 seconds, 239 milliseconds)          
   [info] - Run SparkPi with env and mount secrets. (20 seconds, 863 milliseconds)
   [info] - Run PySpark on simple pi.py example (13 seconds, 795 milliseconds)
   [info] - Run PySpark to test a pyfiles example (15 seconds, 894 milliseconds)
   [info] - Run PySpark with memory customization (13 seconds, 816 milliseconds)
   [info] - Run in client mode. (8 seconds, 186 milliseconds)
   [info] - Start pod creation from template (11 seconds, 870 milliseconds)
   [info] - SPARK-38398: Schedule pod creation from template (12 seconds, 814 milliseconds)
   [info] - Test basic decommissioning (47 seconds, 163 milliseconds)
   [info] - Test basic decommissioning with shuffle cleanup (47 seconds, 300 milliseconds)
   [info] - Test decommissioning with dynamic allocation & shuffle cleanups (2 minutes, 46 seconds)
   [info] - Test decommissioning timeouts (48 seconds, 461 milliseconds)
   [info] - SPARK-37576: Rolling decommissioning (1 minute, 8 seconds)
   [info] - Run SparkPi with volcano scheduler (11 seconds, 852 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enable) (17 seconds, 300 milliseconds)
   [info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enable) (34 seconds, 525 milliseconds)
   [info] - SPARK-38189: Run SparkPi Jobs with priorityClassName (21 seconds, 146 milliseconds)
   [info] - SPARK-38189: Run driver job to validate priority order (19 seconds, 278 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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820176913



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala
##########
@@ -50,10 +51,15 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
 
     queue.foreach(podGroup.editOrNewSpec().withQueue(_).endSpec())
 
+    priorityClassName.foreach(podGroup.editOrNewSpec().withPriorityClassName(_).endSpec())

Review comment:
       Note: here is the core modification. Besides this file, all left files are for test. 




-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1057711510


   To help you make your Volcano PR, I made a `Pod`-spec `priorityClassName` test case PR independently.
   - https://github.com/apache/spark/pull/35716


-- 
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 #35639: [WIP][SPARK-38188][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r813670391



##########
File path: docs/running-on-kubernetes.md
##########
@@ -1356,6 +1356,16 @@ See the [configuration page](configuration.html) for information on Spark config
   </td>
   <td>3.3.0</td>
 </tr>
+<tr>
+  <td><code>spark.kubernetes.job.priorityClassName</code></td>
+  <td><code>(none)</code></td>
+  <td>
+    The name of the queue to which the job is submitted.

Review comment:
       `queue` ? looks like a copy/paste error

##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -292,6 +292,14 @@ private[spark] object Config extends Logging {
       .stringConf
       .createOptional
 
+  val KUBERNETES_JOB_PRIORITY = ConfigBuilder("spark.kubernetes.job.priorityClassName")
+    .doc("The name of the queue to which the job is submitted. This info " +

Review comment:
       again the doc talks about `queue` (https://volcano.sh/en/docs/vcjob/#queue) while it should be https://volcano.sh/en/docs/vcjob/#priorityclassname




-- 
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 #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1056903085


   @holdenk @dongjoon-hyun OK, it's also fine for me, I'd like to add a guide doc to help users enable priority scheduling. I will add 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 commented on a change in pull request #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180270



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       After this merged: https://github.com/apache/spark/pull/35733, :)
   
   Then we can cleanup the end deletions. I will add more notes.




-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059899322


   @viirya Comments 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] Yikun commented on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059641921


   @dongjoon-hyun Got 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] dongjoon-hyun commented on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1058421664


   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] dongjoon-hyun commented on pull request #35639: [WIP][SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059674397


   I merged https://github.com/apache/spark/pull/35728 to master/branch-3.2 for Apache Spark 3.2.2.


-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180270



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       after this merged: https://github.com/apache/spark/pull/35733
   
   we can cleanup this yaml.




-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820177558



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -201,4 +319,21 @@ private[spark] object VolcanoTestsSuite extends SparkFunSuite {
     getClass.getResource("/volcano/disable-queue0-enable-queue1.yml").getFile
   ).getAbsolutePath
   val GROUP_PREFIX = "volcano-test" + UUID.randomUUID().toString.replaceAll("-", "")
+  val VOLCANO_PRIORITY_YAML
+    = new File(getClass.getResource("/volcano/priorityClasses.yml").getFile).getAbsolutePath
+  val HIGH_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/high-priority-driver-template.yml").getFile
+  ).getAbsolutePath
+  val MEDIUM_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/medium-priority-driver-template.yml").getFile
+  ).getAbsolutePath
+  val LOW_PRIORITY_DRIVER_TEMPLATE = new File(
+    getClass.getResource("/volcano/low-priority-driver-template.yml").getFile
+  ).getAbsolutePath

Review comment:
       Hm, where did you use `LOW_PRIORITY_DRIVER_TEMPLATE`, `MEDIUM_PRIORITY_DRIVER_TEMPLATE` and `HIGH_PRIORITY_DRIVER_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 pull request #35639: [SPARK-38189][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1059886760


   @dongjoon-hyun @holdenk 
   
   - Volcano support Pod priority scheduling by default (by creating a temp podgroup in volcano internally), we can only set schedulerName to `vocano` and don't specified feature step. 
   - (This PR) If we want to support the priority with collaborative work with queue/minRes(comming soon). We need to interit the driver pod priorityClassName as job(podgroup) priority.


-- 
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 #35639: [SPARK-38423][K8S] Support priority scheduling with Volcano implementations

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #35639:
URL: https://github.com/apache/spark/pull/35639#discussion_r820180270



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala
##########
@@ -190,6 +238,76 @@ private[spark] trait VolcanoTestsSuite { k8sSuite: KubernetesSuite =>
     }
     deleteYAMLResource(VOLCANO_ENABLE_Q0_AND_Q1_YAML)
   }
+
+  test("SPARK-38189: Run SparkPi Jobs with priorityClassName", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    val priorities = Seq("low", "medium", "high")
+    val groupName = s"$GROUP_PREFIX-priority"
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(groupName),
+          driverTemplate = Option(templatePath)
+        )
+      }
+    }
+    // Make sure all jobs are Succeeded
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+        val pods = getPods(role = "driver", groupName, statusPhase = "Succeeded")
+        assert(pods.size === priorities.size)
+    }
+    // TODO: rebase this
+    deleteYAMLResource(VOLCANO_PRIORITY_YAML)
+  }
+
+  test("SPARK-38189: Run driver job to validate priority order", k8sTestTag, volcanoTag) {
+    // Prepare the priority resource
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    createOrReplaceYAMLResource(VOLCANO_PRIORITY_YAML)
+    // Submit 3 jobs with different priority
+    val priorities = Seq("low", "medium", "high")
+    priorities.foreach { p =>
+      Future {
+        val templatePath = new File(
+          getClass.getResource(s"/volcano/$p-priority-driver-template.yml").getFile
+        ).getAbsolutePath
+        runJobAndVerify(
+          p, groupLoc = Option(s"$GROUP_PREFIX-$p"),
+          queue = Option("queue"),
+          driverTemplate = Option(templatePath),
+          isDriverJob = true
+        )
+      }
+    }
+    // Make sure 3 jobs are pending
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Pending")
+        assert(pods.size === 1)
+      }
+    }
+    // Enable queue to let job enqueue
+    createOrReplaceYAMLResource(ENABLE_QUEUE)
+    Eventually.eventually(TIMEOUT, INTERVAL) {
+      var m = Map.empty[String, Instant]
+      priorities.foreach { p =>
+        val pods = getPods(role = "driver", s"$GROUP_PREFIX-$p", statusPhase = "Succeeded")
+        val conditions = pods.head.getStatus.getConditions.asScala
+        val scheduledTime
+          = conditions.filter(_.getType === "PodScheduled").head.getLastTransitionTime
+        m += (p -> Instant.parse(scheduledTime))
+      }
+      assert(m("high").isBefore(m("medium")))
+      assert(m("medium").isBefore(m("low")))
+    }
+    // TODO: rebase this

Review comment:
       after this merged: https://github.com/apache/spark/pull/35733
   
   We can cleanup the end deletions. will add more notes.




-- 
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 #35639: [SPARK-38423][K8S] Reuse driver pod's `priorityClassName` for `PodGroup`

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35639:
URL: https://github.com/apache/spark/pull/35639#issuecomment-1060257610


   @dongjoon-hyun @viirya Thanks for your review, rebased.


-- 
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