Add a closing method with timeout for JDBC connection #5111
Add a closing method with timeout for JDBC connection #5111bowenliang123 wants to merge 2 commits intoapache:masterfrom
Conversation
bowenliang123
changed the title
bowenliang123
changed the title
bowenliang123
changed the title
Codecov Report
@@ Coverage Diff @@
## master #5111 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 564 564
Lines 31293 31313 +20
Branches 4095 4096 +1
======================================
- Misses 31293 31313 +20
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
cc @jeanlyn |
| } catch (Exception e) { | ||
| LOG.error("Error while closing connection with in timeout", e); | ||
| } finally { | ||
| executor.shutdownNow(); |
There was a problem hiding this comment.
I don't think it works, the body of runWithTimeout runs in another thread, which means the current thread would reach this line soon.
BTW, I don't think we should add such a method at the JDBC driver level, it's the caller's responsibility to cancel after a certain time.
There was a problem hiding this comment.
Once we abandoned a jammed JDBC connection on client side, we want to release and destroy the connection and the member instances inside the connection, like the transport and the client. How to achieve that? And what's cancel for a JDBC connection?
There was a problem hiding this comment.
Once we abandoned a jammed JDBC connection on client side, we want to release and destroy the connection and the member instances inside the connection, like the transport and the client. How to achieve that?
Abandon the whole Connection instance, and make sure to call Connection#close before losing reference.
And what's
cancelfor a JDBC connection?
typo, I mean close
There was a problem hiding this comment.
Or let's add a dedicated method for closing transport inside the KyuubiConnection , in my case for forcibly destroying the instance of TTransport and TCLIService inside?
3 participants
Why are the changes needed?
KyuubiConnectionfor forcibly closing the connection with timeouttransportandclientHow was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request