Skip to content

[SPARK-59914][K8S] Make ExecutorResizePlugin skip executor pods with the placeholder EXECID label - #59175

Closed
dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-59914
Closed

dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-59914

Conversation

@dongjoon-hyun

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR aims to make ExecutorResizePlugin skip executor pods whose spark-exec-id label is the placeholder EXECID, like ExecutorPodsSnapshot does.

Why are the changes needed?

deployment and statefulset pods allocators create executor pods with the EXECID label and patch the real executor ID later on a best-effort basis. Currently, ExecutorResizePlugin runs only with the direct pods allocator, so this is a defensive improvement. It keeps the placeholder out of the cappedExecutors keys and the logs.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CIs with the newly added test case.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 5.5

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

cc @peter-toth

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Could you review this PR, @szehon-ho ?

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Thank you, @HyukjinKwon !

dongjoon-hyun added a commit that referenced this pull request Oct 1, 2026
…h the placeholder `EXECID` label

### What changes were proposed in this pull request?

This PR aims to make `ExecutorResizePlugin` skip executor pods whose `spark-exec-id` label is the placeholder `EXECID`, like `ExecutorPodsSnapshot` does.

### Why are the changes needed?

`deployment` and `statefulset` pods allocators create executor pods with the `EXECID` label and patch the real executor ID later on a best-effort basis. Currently, `ExecutorResizePlugin` runs only with the `direct` pods allocator, so this is a defensive improvement. It keeps the placeholder out of the `cappedExecutors` keys and the logs.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs with the newly added test case.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 5.5

Closes #59175 from dongjoon-hyun/SPARK-59914.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6ed637f)
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Merge Summary:

Posted by merge_spark_pr.py

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

This landed at

@dongjoon-hyun
dongjoon-hyun deleted the SPARK-59914 branch October 1, 2026 00:57
@szehon-ho

Copy link
Copy Markdown
Member

late lgtm

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Thank you, @szehon-ho , too!

dongjoon-hyun added a commit that referenced this pull request Oct 1, 2026
…sizePlugin`

### What changes were proposed in this pull request?

This PR aims to support `ExecutorResizePlugin` with the `deployment` pods allocator for Apache Spark 4.4.0 in addition to the default `direct` pods allocator.

### Why are the changes needed?

[SPARK-56670](#55615) restricted `ExecutorResizePlugin` to the `direct` pods allocator because the higher-level controller's pod template is the source of truth for the other allocators with different mechanism.

However, after SPARK-59914, Spark becomes capable of support `Deployment` because we never updates its pod template, and the `ReplicaSet` controller neither reverts in-place pod resizes nor rolls out a new revision for them.
- #59175

### Does this PR introduce _any_ user-facing change?

Yes. In Apache Spark 4.2.0, `ExecutorResizePlugin` logs a warning and does not start with the `deployment` pods allocator. After this PR, it works with the `deployment` pods allocator.

### How was this patch tested?

Pass the CIs with the updated test case.

I also verified manually on K8s v1.37.0 that the executor pods of the `Deployment` are resized in place up to `spark.kubernetes.executor.resizeMaxMemory` without restarts, while the `Deployment` keeps its pod template and a single `ReplicaSet`.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 5.5

Closes #59180 from dongjoon-hyun/dongjoon/executor-resize-plugin-deployment-0e6321.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun added a commit that referenced this pull request Oct 1, 2026
…sizePlugin`

### What changes were proposed in this pull request?

This PR aims to support `ExecutorResizePlugin` with the `deployment` pods allocator for Apache Spark 4.4.0 in addition to the default `direct` pods allocator.

### Why are the changes needed?

[SPARK-56670](#55615) restricted `ExecutorResizePlugin` to the `direct` pods allocator because the higher-level controller's pod template is the source of truth for the other allocators with different mechanism.

However, after SPARK-59914, Spark becomes capable of support `Deployment` because we never updates its pod template, and the `ReplicaSet` controller neither reverts in-place pod resizes nor rolls out a new revision for them.
- #59175

### Does this PR introduce _any_ user-facing change?

Yes. In Apache Spark 4.2.0, `ExecutorResizePlugin` logs a warning and does not start with the `deployment` pods allocator. After this PR, it works with the `deployment` pods allocator.

### How was this patch tested?

Pass the CIs with the updated test case.

I also verified manually on K8s v1.37.0 that the executor pods of the `Deployment` are resized in place up to `spark.kubernetes.executor.resizeMaxMemory` without restarts, while the `Deployment` keeps its pod template and a single `ReplicaSet`.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 5.5

Closes #59180 from dongjoon-hyun/dongjoon/executor-resize-plugin-deployment-0e6321.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 913175e)
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants