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 21:58:49 UTC
[PR] [SPARK-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
dongjoon-hyun opened a new pull request, #59112:
URL: https://github.com/apache/spark/pull/59112
### What changes were proposed in this pull request?
This PR aims to prune the PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin`.
- Exclude inactive executor pods (`spark-exec-inactive=true`) from the pod listing, like `ExecutorPodsPollingSnapshotSource` and `ExecutorPodsWatchSnapshotSource`.
- Drop the reports of executors without a listed pod, like `ExecutorResizePlugin` does for `cappedExecutors`.
### Why are the changes needed?
`latestReports` is never pruned. So, in long-running applications with dynamic allocation, the reports of removed executors accumulate forever and the whole map is logged at INFO level every interval.
In addition, with `spark.kubernetes.executor.deleteOnTermination=false`, the stale reports of terminated executor pods keep being applied to their PVCs, which can be reused by new executors.
### Does this PR introduce _any_ user-facing change?
Yes but previous it was never pruned. This is a bug fix for `ExecutorPVCResizePlugin`, which was released in [v4.2.0](https://github.com/apache/spark/releases/tag/v4.2.0) (2026-07-11).
### How was this patch tested?
Pass the CIs with the newly added test cases.
### 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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #59112:
URL: https://github.com/apache/spark/pull/59112#discussion_r4133650258
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPVCResizePlugin.scala:
##########
@@ -121,23 +121,28 @@ class ExecutorPVCResizeDriverPlugin extends DriverPlugin with Logging {
}
private[k8s] def checkAndResizePVCs(): Unit = {
- logInfo(s"Latest PVC usage reports: $latestReports")
val appId = sparkContext.applicationId
sparkContext.schedulerBackend match {
case b: KubernetesClusterSchedulerBackend =>
val client = b.kubernetesClient
+ // Skip terminated pods kept by deleteOnTermination=false since their reports are stale.
val pods = client.pods()
.inNamespace(namespace)
.withLabel(SPARK_APP_ID_LABEL, appId)
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
+ .withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
.list()
.getItems.asScala
val podByExecId = pods.flatMap { p =>
Option(p.getMetadata.getLabels.get(SPARK_EXECUTOR_ID_LABEL)).map(_ -> p)
}.toMap
+ // Drop reports of executors without a pod so that latestReports does not grow unbounded.
+ latestReports.keySet().retainAll(podByExecId.keySet.asJava)
Review Comment:
Thank you for the review, @sarutak. This window doesn't apply to live executors. An executor sends its first report only after one full interval because the initial delay of its `scheduleAtFixedRate` is `interval` (a positive multiple of 5 minutes), so its pod is already in the listing by then. Only the reports of executors without an active pod are pruned here, which is intended. So, I'd like to keep the code as is.
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPVCResizePluginSuite.scala:
##########
@@ -308,6 +309,27 @@ class ExecutorPVCResizePluginSuite
verify(pvcsWithNamespace, never()).withName(org.mockito.ArgumentMatchers.anyString())
}
+ test("SPARK-59839: Reports of executors without a pod are removed") {
+ val plugin = createPlugin()
+ val pod = createPodWithPVC(1, "pvc-1", "/data")
+ when(podList.getItems).thenReturn(Collections.singletonList(pod))
+ plugin.receive(PVCDiskUsageReport("1", 0.5))
+ plugin.receive(PVCDiskUsageReport("2", 0.5)) // No pod for executor 2
+
+ plugin.checkAndResizePVCs()
+
+ assert(plugin.latestReports.keySet() === Collections.singleton("1"))
Review Comment:
Thank you. I added the pre-prune assertion.
--
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #59112: [SPARK-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin`
URL: https://github.com/apache/spark/pull/59112
--
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #59112:
URL: https://github.com/apache/spark/pull/59112#discussion_r4130468434
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPVCResizePlugin.scala:
##########
@@ -121,23 +121,28 @@ class ExecutorPVCResizeDriverPlugin extends DriverPlugin with Logging {
}
private[k8s] def checkAndResizePVCs(): Unit = {
- logInfo(s"Latest PVC usage reports: $latestReports")
val appId = sparkContext.applicationId
sparkContext.schedulerBackend match {
case b: KubernetesClusterSchedulerBackend =>
val client = b.kubernetesClient
+ // Skip terminated pods kept by deleteOnTermination=false since their reports are stale.
val pods = client.pods()
.inNamespace(namespace)
.withLabel(SPARK_APP_ID_LABEL, appId)
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
+ .withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
.list()
.getItems.asScala
val podByExecId = pods.flatMap { p =>
Option(p.getMetadata.getLabels.get(SPARK_EXECUTOR_ID_LABEL)).map(_ -> p)
}.toMap
+ // Drop reports of executors without a pod so that latestReports does not grow unbounded.
+ latestReports.keySet().retainAll(podByExecId.keySet.asJava)
Review Comment:
[receive()](https://github.com/apache/spark/pull/59112/changes#diff-5fa000711e41475041f5991251a1d462e02a76b1d0c391b102ab84d1dbf723ccL111) calls `latestReports.put(...)` from the RPC callback thread, while `checkAndResizePVCs()` runs on the single `pvc-resize-plugin` scheduled thread. There is a small window where a brand-new executor reports (`put`) after the `client.pods()...list()` snapshot is taken but before this `retainAll`, so its very first report can be pruned here. This is benign (the executor re-reports every interval, so it self-heals with at most a one-interval delay, and the `ConcurrentHashMap` bulk op stays structurally safe), but a one-line comment noting that the race is intentionally acceptable would help future readers.
```scala
// A newly reported executor may be pruned here if its pod is not yet in the
// listing; harmless because the executor re-reports every interval.
latestReports.keySet().retainAll(podByExecId.keySet.asJava)
```
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPVCResizePluginSuite.scala:
##########
@@ -308,6 +309,27 @@ class ExecutorPVCResizePluginSuite
verify(pvcsWithNamespace, never()).withName(org.mockito.ArgumentMatchers.anyString())
}
+ test("SPARK-59839: Reports of executors without a pod are removed") {
+ val plugin = createPlugin()
+ val pod = createPodWithPVC(1, "pvc-1", "/data")
+ when(podList.getItems).thenReturn(Collections.singletonList(pod))
+ plugin.receive(PVCDiskUsageReport("1", 0.5))
+ plugin.receive(PVCDiskUsageReport("2", 0.5)) // No pod for executor 2
+
+ plugin.checkAndResizePVCs()
+
+ assert(plugin.latestReports.keySet() === Collections.singleton("1"))
Review Comment:
The post-prune assertion (`=== Collections.singleton("1")`) verifies the live report survives. It would be a bit more robust to also assert that both `"1"` and `"2"` exist before `checkAndResizePVCs()`, so that replacing the `retainAll` with an unconditional `clear()` (or dropping the live entry) is clearly caught.
Minor.
```scala
assert(plugin.latestReports.keySet() === Set("1", "2").asJava) // add: pre-prune
plugin.checkAndResizePVCs()
assert(plugin.latestReports.keySet() === Collections.singleton("1"))
```
--
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59112:
URL: https://github.com/apache/spark/pull/59112#issuecomment-5890694723
Thank you, @peter-toth , @sarutak , @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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59112:
URL: https://github.com/apache/spark/pull/59112#issuecomment-5882456643
Could you review this PR when you have some time, @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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59112:
URL: https://github.com/apache/spark/pull/59112#issuecomment-5890671383
Thank you, @sarutak. I updated the PR description to mention that the PVC-name-keyed maps are intentionally out of scope.
--
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #59112:
URL: https://github.com/apache/spark/pull/59112#discussion_r4136900099
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPVCResizePlugin.scala:
##########
@@ -121,23 +121,28 @@ class ExecutorPVCResizeDriverPlugin extends DriverPlugin with Logging {
}
private[k8s] def checkAndResizePVCs(): Unit = {
- logInfo(s"Latest PVC usage reports: $latestReports")
val appId = sparkContext.applicationId
sparkContext.schedulerBackend match {
case b: KubernetesClusterSchedulerBackend =>
val client = b.kubernetesClient
+ // Skip terminated pods kept by deleteOnTermination=false since their reports are stale.
val pods = client.pods()
.inNamespace(namespace)
.withLabel(SPARK_APP_ID_LABEL, appId)
.withLabel(SPARK_ROLE_LABEL, SPARK_POD_EXECUTOR_ROLE)
+ .withoutLabel(SPARK_EXECUTOR_INACTIVE_LABEL, "true")
.list()
.getItems.asScala
val podByExecId = pods.flatMap { p =>
Option(p.getMetadata.getLabels.get(SPARK_EXECUTOR_ID_LABEL)).map(_ -> p)
}.toMap
+ // Drop reports of executors without a pod so that latestReports does not grow unbounded.
+ latestReports.keySet().retainAll(podByExecId.keySet.asJava)
Review Comment:
Got it. Thank you @dongjoon-hyun.
--
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59112:
URL: https://github.com/apache/spark/pull/59112#issuecomment-5879548103
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-59839][K8S] Prune PVC usage reports of removed and inactive executors in `ExecutorPVCResizePlugin` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59112:
URL: https://github.com/apache/spark/pull/59112#issuecomment-5890785325
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/0ccd72732e2d2c98b52a24c7503a97274e35bcba
- merged into branch-4.x https://github.com/apache/spark/commit/a9c1f6598a35b3efd72a8b6a567c895bcab35d47
- merged into branch-4.3 https://github.com/apache/spark/commit/1c8639854733deb5d05d2f12573ae41c83cecb33
- merged into branch-4.2 https://github.com/apache/spark/commit/7b55d54f34d97d704fdc90eba91d2675ac4ea9ca
*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