You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dongjoon-hyun (via GitHub)" <gi...@apache.org> on 2026/09/28 22:10:35 UTC
[PR] [SPARK-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
dongjoon-hyun opened a new pull request, #59113:
URL: https://github.com/apache/spark/pull/59113
### What changes were proposed in this pull request?
This PR aims to fix `ExecutorResizePlugin` to exclude inactive executor pods like `ExecutorPodsPollingSnapshotSource` and `ExecutorPodsWatchSnapshotSource`.
### Why are the changes needed?
When `spark.kubernetes.executor.deleteOnTermination` is `false`, terminated executor pods are kept with the `spark-exec-inactive=true` label. However, `ExecutorResizePlugin` still sends metrics API requests for them at every interval, and these requests fail.
### Does this PR introduce _any_ user-facing change?
No. This only removes the unnecessary K8s API requests for inactive executor pods.
### 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
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #59113: [SPARK-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods
URL: https://github.com/apache/spark/pull/59113
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59113:
URL: https://github.com/apache/spark/pull/59113#issuecomment-5890632602
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/916fba029c86a1533e8af313cfb96f38b58932bf
- merged into branch-4.x https://github.com/apache/spark/commit/bc5f163c26c47664f76a3dbee756f18807d8eb45
- merged into branch-4.3 https://github.com/apache/spark/commit/8e0bf2c06d4b998fb365934694e8c0272836d82a
- merged into branch-4.2 https://github.com/apache/spark/commit/42d9be46f6f6253a7750935b82a4ced01bee3260
*Posted by `merge_spark_pr.py`*
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #59113:
URL: https://github.com/apache/spark/pull/59113#discussion_r4133591614
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorResizePlugin.scala:
##########
@@ -104,6 +104,7 @@ class ExecutorResizeDriverPlugin extends DriverPlugin with Logging {
.inNamespace(namespace)
.withLabel(SPARK_APP_ID_LABEL, appId)
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
+ .withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
Review Comment:
Thank you for the review, @peter-toth. `ExecutorPVCResizePlugin` is handled separately in #59112 (SPARK-59839), which you already approved. It adds the same `withoutLabel` exclusion and also prunes `latestReports` of executors without a listed pod.
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59113:
URL: https://github.com/apache/spark/pull/59113#issuecomment-5879667558
cc @peter-toth
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #59113:
URL: https://github.com/apache/spark/pull/59113#discussion_r4130623815
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorResizePlugin.scala:
##########
@@ -104,6 +104,7 @@ class ExecutorResizeDriverPlugin extends DriverPlugin with Logging {
.inNamespace(namespace)
.withLabel(SPARK_APP_ID_LABEL, appId)
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
+ .withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
Review Comment:
**Finding 1.** `ExecutorPVCResizePlugin.checkAndResizePVCs` has the same pod listing without this line (`ExecutorPVCResizePlugin.scala:130-135`), and there it costs more than a failed API call. `latestReports` is only ever written (`:113`), so a dead executor's last report stays forever. With `deleteOnTermination=false`, its pod is still listed, so `podByExecId` finds it. Each interval then runs `tryResize` again on the same stale ratio. When the previous expansion has finished, `current == capacity`, `newSize = current * 1.1` differs from `requestedSizes`, and the PVC is patched again. `resizeMaxStorage` defaults to `Long.MaxValue`, so nothing stops it.
I checked it in `ExecutorPVCResizePluginSuite` with an inactive-labelled pod, one `0.95` report and three intervals, where the mocked PVC shows each previous resize as done:
```
patches=3 sizes=1100000000,1210000000,1331000000
```
The same one-liner there closes it:
```scala
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
.withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
.list()
```
With it applied, plus the matching `withoutLabel` stub in the suite's `before`, both plugin suites stay green (31 tests). Dropping the report when the executor goes away would also cover the short window before the pod is labelled inactive. Either fix is fine here or as a follow-up.
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59113:
URL: https://github.com/apache/spark/pull/59113#issuecomment-5890578479
Thank you so much, @sarutak , @peter-toth , @HyukjinKwon !
--
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-59840][K8S] Fix `ExecutorResizePlugin` to exclude inactive executor pods [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59113:
URL: https://github.com/apache/spark/pull/59113#issuecomment-5883170502
Could you review this PR when you have some time, @sunchao ?
--
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