Skip to content

Conversation

@turboFei
Copy link
Member

@turboFei turboFei commented Apr 4, 2025

Why are the changes needed?

Currently, if the kyuubi session between client and kyuubi session disconnected without closing properly, it is difficult to debug, and we have to check the kyuubi server log.

It is better that, we can record such kind of information into kyuubi session event.

How was this patch tested?

IT.

image

Was this patch authored or co-authored using generative AI tooling?

No.

@turboFei turboFei changed the title disconnect Record the session disconnected into kyuubi session event Apr 4, 2025
@turboFei turboFei requested a review from pan3793 April 4, 2025 00:41
@turboFei turboFei added this to the v1.10.2 milestone Apr 4, 2025
@turboFei turboFei changed the title Record the session disconnected into kyuubi session event Record the session disconnected info into kyuubi session event Apr 4, 2025
.map(_.asInstanceOf[KyuubiSessionImpl])
.flatMap(_.getSessionEvent).foreach { sessionEvent =>
sessionEvent.exception = Some(new KyuubiException(
s"Session between client and Kyuubi server disconnected without closing properly"))
Copy link
Member Author

@turboFei turboFei Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer:

override def deleteContext(context: ServerContext, in: TProtocol, out: TProtocol): Unit = {
val handle = context.getSessionHandle
if (handle != null) {
info(s"Session [$handle] disconnected without closing properly, close it now")
try {
val needToClose = be.sessionManager.getSession(handle).conf
.getOrElse(SESSION_CLOSE_ON_DISCONNECT.key, "true").toBoolean
if (needToClose) {
be.closeSession(handle)
} else {
warn(s"Session not actually closed because configuration " +
s"${SESSION_CLOSE_ON_DISCONNECT.key} is set to false")
}
} catch {
case e: KyuubiSQLException =>
error("Failed closing session", e)
}
}
}

@turboFei turboFei self-assigned this Apr 4, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (d6f07a6) to head (e465214).

Files with missing lines Patch % Lines
...e/kyuubi/server/KyuubiTBinaryFrontendService.scala 0.00% 9 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #7015   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         694     694           
  Lines       42753   42762    +9     
  Branches     5820    5821    +1     
======================================
- Misses      42753   42762    +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@turboFei
Copy link
Member Author

turboFei commented Apr 4, 2025

Testing:
image

cc @pan3793

@turboFei
Copy link
Member Author

turboFei commented Apr 8, 2025

cc @pan3793

Copy link
Member

@wForget wForget left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@turboFei turboFei closed this in ec074fd Apr 10, 2025
turboFei added a commit that referenced this pull request Apr 10, 2025
…on event

### Why are the changes needed?

Currently, if the kyuubi session between client and kyuubi session disconnected without closing properly, it is difficult to debug, and we have to check the kyuubi server log.

It is better that, we can record such kind of information into kyuubi session event.
### How was this patch tested?

IT.

<img width="1264" alt="image" src="https://github.com/user-attachments/assets/d2c5b6d0-6298-46ec-9b73-ce648551120c" />

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #7015 from turboFei/disconnect.

Closes #7015

c957092 [Wang, Fei] do not post
e465214 [Wang, Fei] nit
bca7f9b [Wang, Fei] post
1cf6f8f [Wang, Fei] disconnect

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
(cherry picked from commit ec074fd)
Signed-off-by: Wang, Fei <[email protected]>
@turboFei turboFei deleted the disconnect branch April 10, 2025 07:09
@turboFei
Copy link
Member Author

thanks, merged to master and branch-1.10

turboFei added a commit to turboFei/kyuubi that referenced this pull request Aug 27, 2025
… session event (apache#690)

### Why are the changes needed?

Currently, if the kyuubi session between client and kyuubi session disconnected without closing properly, it is difficult to debug, and we have to check the kyuubi server log.

It is better that, we can record such kind of information into kyuubi session event.
### How was this patch tested?

IT.

<img width="1264" alt="image" src="https://github.com/user-attachments/assets/d2c5b6d0-6298-46ec-9b73-ce648551120c" />

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#7015 from turboFei/disconnect.

Closes apache#7015

c957092 [Wang, Fei] do not post
e465214 [Wang, Fei] nit
bca7f9b [Wang, Fei] post
1cf6f8f [Wang, Fei] disconnect

Authored-by: Wang, Fei <[email protected]>

Signed-off-by: Wang, Fei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants