Skip to content

Commit 37641f3

Browse files
committed
[KYUUBI apache#2503][FOLLOWUP] Catch all exceptions for sessions timeout check task
# 🔍 Description apache#2503 followup 1. log some message when executing timeout check task 2. catch all exceptions when checking sessions timeout BTW: for `logSessionCountInfo`, log the session class simple name to provide more insights, such as `KyuubiSessionImpl` or `KyuubiBatchSession`. ## 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 --- # Checklists ## 📝 Author Self Checklist - [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [ ] I have performed a self-review - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [ ] Pull request title is okay. - [ ] No license issues. - [ ] Milestone correctly set? - [ ] Test coverage is ok - [ ] Assignees are selected. - [ ] Minimum number of approvals - [ ] No changes are requested **Be nice. Be informative.** Closes apache#5727 from turboFei/catch_all_exception. Closes apache#2503 d726a84 [fwang12] catch all Authored-by: fwang12 <fwang12@ebay.com> Signed-off-by: fwang12 <fwang12@ebay.com>
1 parent 754b57f commit 37641f3

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
9090
conf: Map[String, String]): Session
9191

9292
protected def logSessionCountInfo(session: Session, action: String): Unit = {
93-
info(s"${session.user}'s session with" +
93+
info(s"${session.user}'s ${session.getClass.getSimpleName} with" +
9494
s" ${session.handle}${session.name.map("/" + _).getOrElse("")} is $action," +
9595
s" current opening sessions $getOpenSessionCount")
9696
}
@@ -303,20 +303,21 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
303303

304304
val checkTask = new Runnable {
305305
override def run(): Unit = {
306+
info(s"Checking sessions timeout, current count: $getOpenSessionCount")
306307
val current = System.currentTimeMillis
307308
if (!shutdown) {
308309
for (session <- handleToSession.values().asScala) {
309-
if (session.lastAccessTime + session.sessionIdleTimeoutThreshold <= current &&
310-
session.getNoOperationTime > session.sessionIdleTimeoutThreshold) {
311-
info(s"Closing session ${session.handle.identifier} that has been idle for more" +
312-
s" than ${session.sessionIdleTimeoutThreshold} ms")
313-
try {
310+
try {
311+
if (session.lastAccessTime + session.sessionIdleTimeoutThreshold <= current &&
312+
session.getNoOperationTime > session.sessionIdleTimeoutThreshold) {
313+
info(s"Closing session ${session.handle.identifier} that has been idle for more" +
314+
s" than ${session.sessionIdleTimeoutThreshold} ms")
314315
closeSession(session.handle)
315-
} catch {
316-
case NonFatal(e) => warn(s"Error closing idle session ${session.handle}", e)
316+
} else {
317+
session.closeExpiredOperations()
317318
}
318-
} else {
319-
session.closeExpiredOperations()
319+
} catch {
320+
case NonFatal(e) => warn(s"Error checking session ${session.handle} timeout", e)
320321
}
321322
}
322323
}

0 commit comments

Comments
 (0)