You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jiangzho (via GitHub)" <gi...@apache.org> on 2026/02/19 21:49:37 UTC

[PR] [SPARK-55611] Add RunningWithPartialCapacity state for applications [spark-kubernetes-operator]

jiangzho opened a new pull request, #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513

   ### What changes were proposed in this pull request?
   
   This PR adds a new `RunningWithPartialCapacity` state to `ApplicationStateSummary` that indicates when a Spark application with dynamic allocation disabled is running with executor count between minimum and maximum thresholds.
   
   ### Why are the changes needed?
   
   When dynamic allocation is disabled, if an application has fewer executors than requested (between min and max), it indicates resource constraints in the cluster. This is a tolerable but suboptimal state that should be visible to operators.
   
   With dynamic allocation enabled, executors scaling between min and max is normal behavior, so `RunningHealthy` is appropriate.
   
   The new state provides better observability into cluster resource constraints affecting application performance.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. Users will now see `RunningWithPartialCapacity` status for applications.
   
   ### How was this patch tested?
   
   Added unit tests in `AppRunningStepTest` covering state transition with different number of executors. Also added unit tests in `ApplicationStateSummaryTest` verifying the new state summary.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No
   
   


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


Re: [PR] [SPARK-55611] Add RunningWithPartialCapacity state for applications [spark-kubernetes-operator]

Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3937792588

   @peter-toth would you mind re-run the failed test ? Seems a blip that caused one of the test not able to finish in 30s - which could be result of slow image pulling or similar latency. Tried to reproduce it but seems it passed at my fork (and if that's the case we may slightly increase the timeout in subsequent commits IMO)


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3957327995

   The failed test seems again timing out (a bit flaky to reproduce) - the 2min deadline maybe a few tight if the image is not yet cached. 
   
   @dongjoon-hyun may we separately increase the timeout threshold a bit, like to 5min ?


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3947755865

   Oh, wait, @jiangzho . 
   
   What happens in the following case?
   - When `SparkApplication` has the same min and max values, `10`.
   - After `SparkApplication` enters `RunningHealthy` with 10 executors.
   - Two executor loss happens and Spark driver is trying to get two additionally, but didn't get yet.
     - In this case, the definition of `RunningWithPartialCapacity` rejects this situation because `8` is less than `min (10)` , isn't it?
     > "Application is running with executor count between minimum and maximum capacity, exclusive.";


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#discussion_r2838703974


##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppRunningStep.java:
##########
@@ -96,4 +111,16 @@ public ReconcileProgress reconcile(
           context, statusRecorder, updatedStatus, completeAndDefaultRequeue());
     }
   }
+
+  /**
+   * Checks if dynamic allocation is enabled for the application.
+   *
+   * @param context The SparkAppContext
+   * @return true if dynamic allocation is enabled, false otherwise
+   */
+  private boolean isDynamicAllocationEnabled(SparkAppContext context) {
+      Map<String, String> sparkConf = context.getSparkConf();

Review Comment:
   Do we need to define this variable? Let's simplify by removing this variable definition.



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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#discussion_r2838701382


##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/context/SparkAppContext.java:
##########
@@ -144,4 +145,13 @@ public Pod getDriverPodSpec() {
   public List<HasMetadata> getDriverResourcesSpec() {
     return getSecondaryResourceSpec().getDriverResources();
   }
+
+  /**
+   * Returns the SparkConf from the driver configuration.
+   *
+   * @return The SparkConf object containing Spark configuration properties.
+   */
+  public Map<String, String> getSparkConf() {

Review Comment:
   Could you spin-off this contribution as a new PR? In addition, please replace all existing `app.getSpec().getSparkConf()`, `sparkApplication2.getSpec().getSparkConf()`, `sparkApplication3.getSpec().getSparkConf()` with this new utility method.



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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3944741799

   > @peter-toth would you mind re-run the failed test ? Seems a blip that caused one of the test not able to finish in 30s - which could be result of slow image pulling or similar latency. Tried to reproduce it but seems it passed at my fork (and if that's the case we may slightly increase the timeout in subsequent commits IMO)
   
   I tried to rerun it a few times. It is ok now.
   
   > As in this scenario (the app is scheduled and progressing, just with reduced parallelism) - I would consider this like a capacity shortfall, from scaling perspective. I would lean towards it comparing with "resources" as the latter is broader, and can imply not only missing executors, but also missing CPU / GPU / memory .etc. What do you think ?
   
   Yeah, I see your point. I'm ok with `PartialCapacity`.


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


Re: [PR] [SPARK-55611] Add RunningWithPartialCapacity state for applications [spark-kubernetes-operator]

Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3937812837

   > I wonder whether RunningWithPartialResources might sound better.
   
   As in this scenario (the app is scheduled and progressing, just with reduced parallelism) - I would consider this like a capacity shortfall, from scaling perspective. I would lean towards it comparing with "resources" as the latter is broader, and can imply not only missing executors, but also missing CPU / GPU / memory .etc. What do you think ?


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


Re: [PR] [SPARK-55611] Add RunningWithPartialCapacity state for applications [spark-kubernetes-operator]

Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3935606146

   Looks like a nice improvement to me, although I'm not sure about the name of the new state. I wonder whether `RunningWithPartialResources` might sound better.
   
   Also, there seems to be a UT failure, @jiangzho, can you please look into it?
   
   cc @dongjoon-hyun as well


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


Re: [PR] [SPARK-55611] Add RunningWithPartialCapacity state for applications [spark-kubernetes-operator]

Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3930548043

   cc @peter-toth can you please help with the review ? 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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3945369862

   Also, @jiangzho , please add this new state into the application transition diagram.
   
   https://github.com/apache/spark-kubernetes-operator/blob/df9d8ac848018744e373e416d3513ee19eeb2263/docs/architecture.md?plain=1#L49-L102


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-4012857996

   This one also used a wrong parent when you create a JIRA ID, @jiangzho .
   
   Please use SPARK-55000.
   
   <img width="534" height="82" alt="Screenshot 2026-03-06 at 08 48 47" src="https://github.com/user-attachments/assets/77ad3586-7060-4317-bddd-6f470d390c43" />
   


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#issuecomment-3957303848

   > Two executor loss happens and Spark driver is trying to get two additionally, but didn't get yet.
   > In this case, the definition of RunningWithPartialCapacity rejects this situation because 8 is less than min (10) , isn't it?
   
   Exactly - since in this case the number of executors is less than `min` - it would fall to `RunningWithBelowThresholdExecutors`, indicating that this is not tolerable according to user configuration. `RunningWithPartialCapacity` is the state that's not fully healthy but tolerable. In this sense, I believe the given description is still accurate


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #513: [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD
URL: https://github.com/apache/spark-kubernetes-operator/pull/513


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


Re: [PR] [SPARK-55611] Add `RunningWithPartialCapacity` state for `SparkApplication` CRD [spark-kubernetes-operator]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #513:
URL: https://github.com/apache/spark-kubernetes-operator/pull/513#discussion_r2838688578


##########
spark-operator-api/src/main/java/org/apache/spark/k8s/operator/status/ApplicationStateSummary.java:
##########
@@ -57,6 +57,12 @@ public enum ApplicationStateSummary implements BaseStateSummary {
    */
   RunningHealthy,
 
+  /**
+   * Application is running with dynamic allocation disabled and executor count is between minimum
+   * and maximum thresholds. The application is in a tolerable state but not at optimal capacity.

Review Comment:
   This is correct mathematically but wrong in the general English sentence. Could you revise like the following, @jiangzho ?
   ```
   - between minimum and maximum thresholds
   + between minimum and maximum thresholds, exclusive
   ```



##########
spark-operator-api/src/main/java/org/apache/spark/k8s/operator/Constants.java:
##########
@@ -164,6 +164,13 @@ public class Constants {
   /** Message indicating that the application is running healthy. */
   public static final String RUNNING_HEALTHY_MESSAGE = "Application is running healthy.";
 
+  /**
+   * Message indicating that the application is running with partial executor capacity (between min
+   * and max with dynamic allocation disabled).

Review Comment:
   ditto.



##########
spark-operator-api/src/main/java/org/apache/spark/k8s/operator/Constants.java:
##########
@@ -164,6 +164,13 @@ public class Constants {
   /** Message indicating that the application is running healthy. */
   public static final String RUNNING_HEALTHY_MESSAGE = "Application is running healthy.";
 
+  /**
+   * Message indicating that the application is running with partial executor capacity (between min
+   * and max with dynamic allocation disabled).
+   */
+  public static final String RUNNING_WITH_PARTIAL_CAPACITY_MESSAGE =
+      "Application is running with executor count between minimum and maximum capacity.";

Review Comment:
   ditto.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org