Uh oh!
There was an error while loading. Please reload this page.
[SPARK-41410][K8S][FOLLOWUP] Remove PVC_COUNTER decrement - #38949
[SPARK-41410][K8S][FOLLOWUP] Remove PVC_COUNTER decrement#38949dongjoon-hyun wants to merge 2 commits into
Conversation
dongjoon-hyun
commented
Dec 7, 2022
cc @tedyu |
dongjoon-hyun
commented
Dec 7, 2022
For reviewers, the following test case is added. |
dongjoon-hyun
commented
Dec 7, 2022
I'll test this PR more in the cluster. |
dongjoon-hyun
commented
Dec 7, 2022
I already suggest you to use my test code to verify your PR, @tedyu . |
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Could you review this follow-up, @viirya ?
| .inNamespace(namespace) | ||
| .resource(createdExecutorPod) | ||
| .delete() | ||
| PVC_COUNTER.decrementAndGet() |
There was a problem hiding this comment.
This is for case 1, right? For case 2, we don't need to do anything as PVC_COUNTER was not changed for that case?
There was a problem hiding this comment.
Yes, right for case 1 and 2. So, this is a complete and minimal patch, @viirya .
There was a problem hiding this comment.
Since this is used for driver-owned PVCs, the deletion of pods is irrelevant to the number of PVC. That was my logical bug at the initial patch.
dongjoon-hyun
commented
Dec 7, 2022
Thank you so much, @viirya . Merged to master. |
### What changes were proposed in this pull request? This is a follow-up to remove `PVC_COUNTER` decrement part to handle exception cases. ### Why are the changes needed? This PR handles the following two cases. 1. If pod creation API throws `KubernetesClientException`, we should not change `PVC_COUNTER`. 2. In case of (1), Spark try to delete pod. If pod deletion API also throws `KubernetesClientException`, we should not change `PVC_COUNTER`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs with the newly added test case. Closesapache#38949 from dongjoon-hyun/SPARK-41410-2. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
What changes were proposed in this pull request?
This is a follow-up to remove
PVC_COUNTERdecrement part to handle exception cases.Why are the changes needed?
This PR handles the following two cases.
KubernetesClientException, we should not changePVC_COUNTER.KubernetesClientException, we should not changePVC_COUNTER.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass the CIs with the newly added test case.