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

Added cancelQueryTimeout to cancel QueryTimeout on Connection and Statement #674

Merged
merged 12 commits into from
May 2, 2018

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Apr 11, 2018

Extension of PR #560.

Adds new property cancelQueryTimeout on both Connection and Statement classes. The Connection level property is used by all statements by default, while setting it at Statement level overrides the connection property.

The fix will address issue #525 where queryTimeout hangs and does not throw exception due to silent dropping of TCP connection.

The driver now reads timeout values from statement being executed and not connection (Changed from #560 initial implementation) and shall wait the total amount of cancelQueryTimeout + queryTimeout to drop the connection and close the channel.

Note: The new connection property of cancelQueryTimeout will be effective only if the queryTimeout is set in the driver.

@codecov-io
Copy link

codecov-io commented Apr 11, 2018

Codecov Report

Merging #674 into dev will decrease coverage by 0.04%.
The diff coverage is 59.72%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #674      +/-   ##
============================================
- Coverage     48.22%   48.18%   -0.05%     
+ Complexity     2579     2578       -1     
============================================
  Files           113      113              
  Lines         26569    26626      +57     
  Branches       4438     4449      +11     
============================================
+ Hits          12813    12829      +16     
- Misses        11619    11656      +37     
- Partials       2137     2141       +4
Flag Coverage Δ Complexity Δ
#JDBC42 48.07% <59.72%> (-0.05%) 2574 <4> (+1)
#JDBC43 48.04% <59.72%> (-0.08%) 2568 <4> (-4)
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerResource.java 100% <ø> (ø) 4 <0> (ø) ⬇️
.../microsoft/sqlserver/jdbc/SQLServerDataSource.java 45.19% <0%> (-0.48%) 66 <0> (ø)
.../com/microsoft/sqlserver/jdbc/SQLServerDriver.java 76.98% <100%> (+0.12%) 25 <0> (ø) ⬇️
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 50.85% <100%> (ø) 157 <0> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.67% <100%> (+0.12%) 240 <0> (+1) ⬆️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 45.79% <40%> (-0.04%) 289 <1> (+2)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 33.14% <50%> (ø) 245 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.44% <60%> (-0.18%) 134 <3> (+2)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 55.16% <75%> (+0.16%) 0 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerException.java 75.6% <0%> (-1.63%) 29% <0%> (-1%)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55bc4a0...835fddc. Read the comment docs.

@cheenamalhotra cheenamalhotra modified the milestones: 6.5.1, 6.5.2 Apr 12, 2018
@cheenamalhotra cheenamalhotra added this to Under Peer Review in MSSQL JDBC Apr 12, 2018
…ilent connection drops - Minor fixes | GitHub issue 525
@cheenamalhotra
Copy link
Member Author

Hi @ulvii - made recommended fixes. Please review once more.

Thanks!

@rene-ye rene-ye self-requested a review May 2, 2018 22:52
@cheenamalhotra cheenamalhotra merged commit 914008f into microsoft:dev May 2, 2018
MSSQL JDBC automation moved this from Under Peer Review to Closed/Merged PRs May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

None yet

5 participants