[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340231350 In other words, please revert all changes and remove one line, `PVC_COUNTER.decrementAndGet()`. -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340232151 Okay. Since we don't agree, I will make my PR too. We can compare side-by-side, @tedyu . :) -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340234190 BTW, we need to add the test case to validate the ideas. I'll try to add to my PR. You may can reuse it. -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340236767 Please make a valid est case for your claim. -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340238163 This is handled properly by removing `decrement` line. > the counter shouldn't be decremented. -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340288819 It's totally fine because `spark.kubernetes.driver.reusePersistentVolumeClaim=true`. We can reuse that PVC later, @tedyu . > e.g. the test can produce exception when the following is called: > > ``` > newlyCreatedExecutors(newExecutorId) = (resourceProfileId, clock.getTimeMillis()) > ``` -- 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-unsubscr...@spark.apache.org 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
[GitHub] [spark] dongjoon-hyun commented on pull request #38948: [SPARK-41410][K8S][FOLLOW-UP] Decrement PVC_COUNTER when the pod deletion happens
dongjoon-hyun commented on PR #38948: URL: https://github.com/apache/spark/pull/38948#issuecomment-1340291989 @tedyu . It seems that you forgot `spark.kubernetes.driver.ownPersistentVolumeClaim=true`. Pod deletion doesn't clean up PVCs. It's owned by Driver pod. This is not your bug. That was originated my bug. :) > If exception happens at newlyCreatedExecutors(newExecutorId) = (or later in the try block), the pod would be deleted. Why shouldn't PVC_COUNTER be decremented (note: it has been incremented after PVC creation) ? -- 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-unsubscr...@spark.apache.org 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
