Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KYUUBI #6204] Fix kyuubi session limiter leak when opening session f… #10

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

karjensia
Copy link

…ailed

🔍 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 🔖

  • 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 ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

Closes apache#6204 from turboFei/limiter_leak.

Closes apache#6204

c0f2969 [Wang, Fei] refine
98fda94 [Wang, Fei] fix leak

Authored-by: Wang, Fei [email protected]

🔍 Description

Issue References 🔗

This pull request fixes #

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

  • 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 ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

…sion f…

…ailed

# 🔍 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 🔖

- [ ] 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 ⚰️

#### Behavior With This Pull Request 🎉

#### 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 apache#6204 from turboFei/limiter_leak.

Closes apache#6204

c0f2969 [Wang, Fei] refine
98fda94 [Wang, Fei] fix leak

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
@karjensia karjensia requested review from minyk and cyon13 April 3, 2024 08:12
@karjensia karjensia self-assigned this Apr 3, 2024
@karjensia karjensia merged commit 3686ebe into nexr:branch-1.8.1 Apr 4, 2024
@karjensia karjensia deleted the BRD-patch branch April 4, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants