dongjoon-hyun commented on a change in pull request #32564:
URL: https://github.com/apache/spark/pull/32564#discussion_r633258964



##########
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala
##########
@@ -125,6 +125,14 @@ private[spark] class ExecutorPodsAllocator(
     newlyCreatedExecutors --= k8sKnownExecIds
     schedulerKnownNewlyCreatedExecs --= k8sKnownExecIds
 
+    // Although we are going to delete some executors due to timeout in this 
function,
+    // it takes undefined time before the actual deletion. Hence, we should 
collect all PVCs
+    // in use at the beginning. False positive is okay in this context in 
order to be safe.

Review comment:
       Spark only gets `snapshots` here. So, the snapshot cannot be the same 
with K8s AS-IS status. Always, there is a delay and some deleted executors will 
be notified at the next snapshot by being removed from the snapshot.
   > both in-use and not in-use (from removed executors)?
   
   In addition, we cannot reclaim the PVC in the `Terminating`-status pods. 
Those PVCs are still in the snapshot and connected to the pods.




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

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



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

Reply via email to