guangyy opened a new pull request, #59995: URL: https://github.com/apache/spark/pull/59995
### What changes were proposed in this pull request? `ExecutorPodsAllocator.onNewSnapshots` builds the set of in-use PVCs (`k8sKnownPVCNames`) that `getReusablePVCs` uses to decide which driver-owned PVCs are free. That set was built only from the snapshots delivered in the current batch. This PR builds it from: - the executor pods in `lastSnapshot`, - the pods in the current batch (`aggregatedPods`), and - the PVC claim names of executor pods that were requested from Kubernetes but have not appeared in any snapshot yet. A new map, `newlyCreatedExecutorPVCs`, records these when each pod is created and prunes them together with `newlyCreatedExecutors` / `schedulerKnownNewlyCreatedExecs`. ### Why are the changes needed? With `spark.kubernetes.driver.ownPersistentVolumeClaim=true` and `spark.kubernetes.driver.reusePersistentVolumeClaim=true` (both default true), a new executor pod can be given a PVC that a running executor still mounts: 1. `SnapshotsSubscriber` calls `onNewSnapshots` every `spark.kubernetes.allocation.batch.delay`, even when no new snapshot arrived. Executor counting still uses `lastSnapshot`, so the allocator may scale up in that round, but the in-use PVC set is empty. Every driver-owned PVC older than `podCreationTimeout` then looks reusable, including mounted ones. 2. A pod created in an earlier round that reused PVC X is not in any snapshot until the watch or poll delivers it, so X can be handed out again in the next round. SPARK-41388 and SPARK-49385 only protect PVCs younger than `podCreationTimeout`, so long-running applications are not covered. With the default ReadWriteOncePod access mode, the new pod can never be scheduled (`VolumeRestrictions`). It stays Pending until dynamic allocation deletes it or the application ends. On a Spark 4.1.1 production application, we saw one PVC referenced by 7 pods and 18 duplicate Pending executor pods. See SPARK-60778 for details. ### Does this PR introduce _any_ user-facing change? No, other than fixing the bug. A PVC that is wrongly treated as in use only causes a new PVC to be created instead of a reused one. ### How was this patch tested? Added a new unit test in `ExecutorPodsAllocatorSuite`: 1. A driver-owned PVC older than `podCreationTimeout` is reused by executor 1, which becomes Running. 2. The target is raised to 2 in a round with no new snapshot. 3. The test asserts that executor 2 gets a newly created PVC. `build/sbt -Pkubernetes "kubernetes/testOnly *ExecutorPodsAllocatorSuite"`: all 31 tests pass. Without the change in `ExecutorPodsAllocator`, the new test fails because no new PVC is created (`pvcWithNamespace.resource` is never invoked), meaning executor 2 reuses the mounted `pvc-0`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5.5 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
