(kyuubi) branch branch-1.7 updated: [KYUUBI #6204] Fix kyuubi session limiter leak when opening session failed

[fe...@apache.org]

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 <fw...@ebay.com>
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 <fw...@ebay.com>
    Signed-off-by: Wang, Fei <fw...@ebay.com>
    (cherry picked from commit e1861d9ded7ddd6178099fbd0c25885e5966083d)
    Signed-off-by: Wang, Fei <fw...@ebay.com>
---
 .../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)