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

feiwang 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 eca9bb0be [KYUUBI #6204] Fix kyuubi session limiter leak when opening 
session failed
eca9bb0be is described below

commit eca9bb0be703a71ca9b12c6f409f4514742be7bf
Author: Wang, Fei <[email protected]>
AuthorDate: Thu Mar 21 22:17:32 2024 -0700

    [KYUUBI #6204] Fix kyuubi session limiter leak when opening session failed
    
    # :mag: Description
    ## Issue References ๐Ÿ”—
    
    This pull request fixes #
    
    ## Describe Your Solution ๐Ÿ”ง
    
    We found that, a user has no active sessions, but kyuubi said that the user 
reach the max limit sessions per user.
    Now, we increase the session limiter for user when opening session and 
decrease it when closing session.
    
    But if the user open session failed, it will not decrease the session 
limiter.
    
    This pr fix session limiter leak issue when failed to open session.
    
    Before open session, add session handle into sessionHandleMap, and invoke 
SessionManager::closeSession when failed to open session.
    
    ## Types of changes :bookmark:
    
    - [ ] Bugfix (non-breaking change which fixes an issue)
    - [ ] New feature (non-breaking change which adds functionality)
    - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
    
    ## Test Plan ๐Ÿงช
    
    #### Behavior Without This Pull Request :coffin:
    
    #### Behavior With This Pull Request :tada:
    
    #### Related Unit Tests
    
    ---
    
    # Checklist ๐Ÿ“
    
    - [x] This patch was not authored or co-authored using [Generative 
Tooling](https://www.apache.org/legal/generative-tooling.html)
    
    **Be nice. Be informative.**
    
    Closes #6204 from turboFei/limiter_leak.
    
    Closes #6204
    
    c0f2969fc [Wang, Fei] refine
    98fda9438 [Wang, Fei] fix leak
    
    Authored-by: Wang, Fei <[email protected]>
    Signed-off-by: Wang, Fei <[email protected]>
    (cherry picked from commit e1861d9ded7ddd6178099fbd0c25885e5966083d)
    Signed-off-by: Wang, Fei <[email protected]>
---
 .../src/main/scala/org/apache/kyuubi/session/SessionManager.scala   | 6 +++---
 .../main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git 
a/kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala 
b/kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
index 4f2fbe2e5..342210cd2 100644
--- 
a/kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
+++ 
b/kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
@@ -103,16 +103,16 @@ abstract class SessionManager(name: String) extends 
CompositeService(name) {
       conf: Map[String, String]): SessionHandle = {
     info(s"Opening session for $user@$ipAddress")
     val session = createSession(protocol, user, password, ipAddress, conf)
+    val handle = session.handle
     try {
-      val handle = session.handle
-      session.open()
       setSession(handle, session)
+      session.open()
       logSessionCountInfo(session, "opened")
       handle
     } catch {
       case e: Exception =>
         try {
-          session.close()
+          closeSession(handle)
         } catch {
           case t: Throwable =>
             warn(s"Error closing session for $user client ip: $ipAddress", t)
diff --git 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala
 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala
index f664614f5..db35b6042 100644
--- 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala
+++ 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala
@@ -154,14 +154,14 @@ class KyuubiSessionManager private (name: String) extends 
SessionManager(name) {
     batchLimiter.foreach(_.increment(UserIpAddress(user, ipAddress)))
     val handle = batchSession.handle
     try {
-      batchSession.open()
       setSession(handle, batchSession)
+      batchSession.open()
       logSessionCountInfo(batchSession, "opened")
       handle
     } catch {
       case e: Exception =>
         try {
-          batchSession.close()
+          closeSession(handle)
         } catch {
           case t: Throwable =>
             warn(s"Error closing batch session[$handle] for $user client ip: 
$ipAddress", t)

Reply via email to