dongjoon-hyun commented on code in PR #966:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/966#discussion_r4267728635
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/kueue/KueueWorkloadFactory.java:
##########
@@ -72,6 +72,13 @@ public final class KueueWorkloadFactory {
public static final String PODSET_MASTER =
Constants.LABEL_SPARK_ROLE_MASTER_VALUE;
public static final String PODSET_WORKER =
Constants.LABEL_SPARK_ROLE_WORKER_VALUE;
+ /**
+ * Prefix of the annotation of a Workload, followed by the name of a pod
set, which holds how many
+ * of its pods must be ready for the Kueue `PodsReady` condition instead of
its count, e.g. the
+ * executors which dynamic allocation keeps, see {@link
KueueWorkloadUtils#recordPodsReady}.
+ */
+ public static final String ANNOTATION_PODS_READY_PREFIX =
"spark.operator/kueue-pods-ready-";
Review Comment:
Done. It's a plain constant now, which also simplifies the in-place update.
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/kueue/KueueWorkloadFactory.java:
##########
@@ -301,17 +321,49 @@ static PodSet buildDriverPodSet(
}
static PodSet buildExecutorPodSet(
- final SparkApplication app, final Map<String, String> sparkConf) {
+ final SparkApplication app,
+ final Map<String, String> sparkConf,
+ final boolean dynamicAllocation) {
Review Comment:
Done. `buildWorkload` now decides the executor count and the annotation
together.
##########
docs/spark_custom_resources.md:
##########
@@ -810,9 +810,20 @@ relates it to the reconciliation.
ones with `kubectl get workloads -A -l spark.operator/spark-app-name` and
`kubectl get workloads -A -l spark.operator/spark-cluster-name`, and delete
only those whose
owner has no running pods, e.g. of a `Failed` `SparkCluster`.
-* Dynamic allocation, a `SparkCluster` with `minWorkers < maxWorkers`, and pod
template files set
- through `spark.kubernetes.{driver,executor}.podTemplateFile` are not
supported yet. Such a
- resource fails with `SchedulingFailure` instead of being queued.
+* A `SparkCluster` with `minWorkers < maxWorkers` and pod template files set
through
+ `spark.kubernetes.{driver,executor}.podTemplateFile` are not supported yet.
Such a resource fails
+ with `SchedulingFailure` instead of being queued.
+* Since Kueue cannot resize the quota of an admitted `Workload` yet, a
`SparkApplication` with
Review Comment:
Split it into sub-bullets.
##########
docs/spark_custom_resources.md:
##########
@@ -810,9 +810,20 @@ relates it to the reconciliation.
ones with `kubectl get workloads -A -l spark.operator/spark-app-name` and
`kubectl get workloads -A -l spark.operator/spark-cluster-name`, and delete
only those whose
owner has no running pods, e.g. of a `Failed` `SparkCluster`.
-* Dynamic allocation, a `SparkCluster` with `minWorkers < maxWorkers`, and pod
template files set
- through `spark.kubernetes.{driver,executor}.podTemplateFile` are not
supported yet. Such a
- resource fails with `SchedulingFailure` instead of being queued.
+* A `SparkCluster` with `minWorkers < maxWorkers` and pod template files set
through
+ `spark.kubernetes.{driver,executor}.podTemplateFile` are not supported yet.
Such a resource fails
+ with `SchedulingFailure` instead of being queued.
+* Since Kueue cannot resize the quota of an admitted `Workload` yet, a
`SparkApplication` with
+ dynamic allocation fails with `SchedulingFailure` unless it is annotated with
+ `spark.operator/kueue-reserve-max-executors: "true"`, which is ignored
without dynamic
+ allocation. As a stopgap, its `Workload` then reserves the quota of
+ `spark.dynamicAllocation.maxExecutors` executors, which must be set since it
is unlimited by
+ default, for the whole attempt: Kueue admits it only once the maximum fits,
and preemption, fair
+ sharing and a `ProvisioningRequest` admission check count the maximum even
while fewer executors
+ run. Like the integration of Kueue for the Kubeflow Spark Operator, the
`PodsReady` condition
Review Comment:
Thanks. Reworded as you suggested.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
