wForget commented on code in PR #4241:
URL: https://github.com/apache/kyuubi/pull/4241#discussion_r1096888880
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -247,7 +250,7 @@ class KyuubiSessionImpl(
try {
if (_client != null) _client.closeSession()
} finally {
- if (engine != null) engine.close()
+ openSessionError.foreach { _ => if (engine != null) engine.close() }
Review Comment:
> We can, but it's a bit complex. I think this pr is enough to fix
regression.
>
> I see your requreiements that to avoid the leaked engine. We can improve
that in a global view rather than wait and kill one by one. e.g. Use a thread
to detect if there is a leaked engine.
>
> Note, it's the key to define what is the `leaked engine`. In general, A
`leaked engine` can be:
>
> * status is finished in resource manager but engine build process is alive
(for all share level)
> * session is closed but status is not fiinshed or engine build process is
alive (for connection share level and batch mode)
>
> We can have a new pr if you are interested in.
I agree with you, this pr is enough to fix the previous issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
