ulysses-you commented on code in PR #4241:
URL: https://github.com/apache/kyuubi/pull/4241#discussion_r1096883352


##########
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 connect share level and batch mode)
   
   We can have a new pr if you are interested in.



-- 
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]

Reply via email to