[kyuubi] branch branch-1.7 updated: [KYUUBI #4816] [K8S] Correct the implementation of cleanup terminated appInfo

[chengpan]

This is an automated email from the ASF dual-hosted git repository.

chengpan pushed a commit to branch branch-1.7
in repository https://gitbox.apache.org/repos/asf/kyuubi.git


The following commit(s) were added to refs/heads/branch-1.7 by this push:
     new 3ed5401e9 [KYUUBI #4816] [K8S] Correct the implementation of cleanup 
terminated appInfo
3ed5401e9 is described below

commit 3ed5401e91977d07ed7426eaca1d974e6355cc8f
Author: fwang12 <fwan...@ebay.com>
AuthorDate: Thu May 11 15:32:25 2023 +0800

    [KYUUBI #4816] [K8S] Correct the implementation of cleanup terminated 
appInfo
    
    ### _Why are the changes needed?_
    
    For `LocalCache`, `put` operation will `remove` the existing element.
    
    
https://github.com/apache/kyuubi/blob/299df0d7c2ad9ad6509861fada2d25ee2933e1fd/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala#L202-L207
    And then trigger the removal listener.
    
    
https://github.com/apache/kyuubi/blob/299df0d7c2ad9ad6509861fada2d25ee2933e1fd/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala#L60-L68
    
    and evict the existing app info from `appInfoStore`.
    
    and then the `kyuubi.kubernetes.terminatedApplicationRetainPeriod` does not 
work.
    
    And then we can only get `NOT_FOUND` application state.
    So, we should check whether there is existing 
`cleanupTerminatedAppInfoTrigger` key before `put`.
    
    ### _How was this patch tested?_
    - [ ] Add some test cases that check the changes thoroughly including 
negative and positive cases if possible
    
    - [x] Add screenshots for manual tests if appropriate
    
    Before:
    <img width="738" alt="image" 
src="https://github.com/apache/kyuubi/assets/6757692/15e6a03e-c0ea-4e40-8b49-a04fa8255dcb";>
    
    After:
    <img width="714" alt="image" 
src="https://github.com/apache/kyuubi/assets/6757692/63994b0c-ceae-46fa-97a5-60f2d6dcf994";>
    
    - [x] [Run 
test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests)
 locally before make a pull request
    
    Closes #4816 from turboFei/mark_if_absent.
    
    Closes #4816
    
    d767756fa [fwang12] put if absent
    
    Authored-by: fwang12 <fwan...@ebay.com>
    Signed-off-by: Cheng Pan <cheng...@apache.org>
    (cherry picked from commit 369c21a61156bf4bf82e8641e2d85eaa5acd8922)
    Signed-off-by: Cheng Pan <cheng...@apache.org>
---
 .../apache/kyuubi/engine/KubernetesApplicationOperation.scala    | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala
 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala
index 399399a03..d6c28d022 100644
--- 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala
+++ 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala
@@ -199,10 +199,11 @@ class KubernetesApplicationOperation extends 
ApplicationOperation with Logging {
         error = Option(pod.getStatus.getReason)))
   }
 
-  private def markApplicationTerminated(pod: Pod): Unit = {
-    cleanupTerminatedAppInfoTrigger.put(
-      pod.getMetadata.getLabels.get(LABEL_KYUUBI_UNIQUE_KEY),
-      toApplicationState(pod.getStatus.getPhase))
+  private def markApplicationTerminated(pod: Pod): Unit = synchronized {
+    val key = pod.getMetadata.getLabels.get(LABEL_KYUUBI_UNIQUE_KEY)
+    if (cleanupTerminatedAppInfoTrigger.getIfPresent(key) == null) {
+      cleanupTerminatedAppInfoTrigger.put(key, 
toApplicationState(pod.getStatus.getPhase))
+    }
   }
 }