Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce JDBC parameters to control connection timeout #3152

Closed
wants to merge 4 commits into from

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Jul 26, 2022

Why are the changes needed?

There are two concepts connect timeout and so timeout in the socket, currently, the Kyuubi Hive JDBC driver uses DriverManager#loginTimeout as both of them, it does not make sense, and will cause unexpected "read timeout" exceptions if DriverManager#loginTimeout was set a small value, e.g. Spring Boot set it to 30s in default.

This PR proposes to introduce two dedicated JDBC parameters to control them.

See also apache/hive#3379

How 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

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #3152 (816a851) into master (8778209) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

❗ Current head 816a851 differs from pull request most recent head 5dcae66. Consider uploading reports for the commit 5dcae66 to get more accurate results

@@             Coverage Diff              @@
##             master    #3152      +/-   ##
============================================
- Coverage     51.44%   51.37%   -0.08%     
  Complexity        6        6              
============================================
  Files           456      456              
  Lines         25328    25336       +8     
  Branches       3520     3520              
============================================
- Hits          13031    13016      -15     
- Misses        11055    11070      +15     
- Partials       1242     1250       +8     
Impacted Files Coverage Δ
.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java 4.16% <0.00%> (-0.05%) ⬇️
...c/main/java/org/apache/kyuubi/jdbc/hive/Utils.java 36.88% <ø> (ø)
.../org/apache/kyuubi/jdbc/hive/auth/ThriftUtils.java 0.00% <0.00%> (ø)
.../apache/kyuubi/client/api/v1/dto/BatchRequest.java 52.94% <0.00%> (-15.69%) ⬇️
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala 78.00% <0.00%> (-2.00%) ⬇️
...pache/kyuubi/engine/KyuubiApplicationManager.scala 82.35% <0.00%> (-1.48%) ⬇️
...ache/kyuubi/operation/KyuubiOperationManager.scala 80.82% <0.00%> (-1.37%) ⬇️
.../engine/spark/session/SparkSQLSessionManager.scala 78.48% <0.00%> (-1.27%) ⬇️
...ain/scala/org/apache/kyuubi/engine/EngineRef.scala 75.23% <0.00%> (-0.96%) ⬇️
...g/apache/kyuubi/operation/BatchJobSubmission.scala 77.01% <0.00%> (-0.63%) ⬇️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@pan3793 pan3793 changed the title [WIP] Introduce JDBC parameters to control connection timeout Introduce JDBC parameters to control connection timeout Jul 27, 2022
@pan3793 pan3793 marked this pull request as ready for review July 27, 2022 11:54
@pan3793 pan3793 requested a review from cxzl25 July 27, 2022 11:54
@pan3793 pan3793 self-assigned this Jul 29, 2022
@pan3793 pan3793 added this to the v1.6.0 milestone Jul 29, 2022
@pan3793
Copy link
Member Author

pan3793 commented Jul 29, 2022

Thanks, merging to master

@pan3793 pan3793 closed this in 65ccf78 Jul 29, 2022
@pan3793 pan3793 deleted the timeout branch July 29, 2022 09:43
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.

3 participants