Skip to content

Improvements + Added test#2

Closed
cheenamalhotra wants to merge 3 commits intosehrope:shared-timer-for-timeoutsfrom
cheenamalhotra:shared-timer-improvements
Closed

Improvements + Added test#2
cheenamalhotra wants to merge 3 commits intosehrope:shared-timer-for-timeoutsfrom
cheenamalhotra:shared-timer-improvements

Conversation

@cheenamalhotra
Copy link
Copy Markdown

Few improvements on the implementation:

  • Removed Abstract Class and Interface (Not required at this point)
  • Generalized TdsTimeoutTask
    (Removed variations of Timeout Task as in both Query Timeout and Bulk Copy Timeout cases the driver must abort connection if timeout has occurred)
  • Added Shared Timer thread test to validate running thread in background. The JUnit tests were cleaned up to ensure all connections are aborted accurately in order to run this test.

sehrope and others added 3 commits January 5, 2019 15:00
Replaces the timeout handling for basic and bulk TDS commands to use a new SharedTimer
class. SharedTimer provides a static method for fetching an existing static object or
creating one on demand. Usage is tracked through reference counting and callers are
required to call removeRef() when they will no longer be using the SharedTimer. If the
SharedTimer does not have any more references then its internal ScheduledThreadPoolExecutor
will be shutdown.

The SharedTimer is cached at the Connection level so that repeated invocations do not
create new timers. Connections only create timers on first use so if no actions involve
a timeout then no timer is fetched or created. If a Connection does create a timer then
it will be released when the Connection closed.

Properly written JDBC applications that always close their Connection objects when
they are finished using them should not have any extra threads running after they are
all closed. Applications that do not use query timeouts will not have any extra threads
created as they are only done on demand.  Applications that use timeouts and use
a JDBC connection pool will have a single shared object across all JDBC connections as
long as there are some open connections in the pool with timeouts enabled.

Interrupt actions to handle a timeout are executed in their own thread. A handler thread is
created when the timeout occurs with the thread name matching the connection id of the
client connection that created the timeout. If the timeout is canceled prior to the interrupt
action being executed, say because the command finished, then no handler thread is created.

Note that the sharing of the timers happens across all Connections, not just Connections
with the same JDBC URL and properties.
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2 into shared-timer-for-timeouts will decrease coverage by 0.11%.
The diff coverage is 66.66%.

Impacted file tree graph

@@                       Coverage Diff                       @@
##             shared-timer-for-timeouts       #2      +/-   ##
===============================================================
- Coverage                        50.37%   50.26%   -0.12%     
+ Complexity                        2931     2921      -10     
===============================================================
  Files                              122      120       -2     
  Lines                            28108    28098      -10     
  Branches                          4692     4694       +2     
===============================================================
- Hits                             14160    14124      -36     
- Misses                           11643    11679      +36     
+ Partials                          2305     2295      -10
Flag Coverage Δ Complexity Δ
#JDBC42 49.85% <66.66%> (-0.12%) 2882 <5> (-12)
#JDBC43 50.22% <66.66%> (-0.07%) 2919 <5> (-5)
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 51.47% <100%> (ø) 255 <0> (ø) ⬇️
...java/com/microsoft/sqlserver/jdbc/SharedTimer.java 73.91% <100%> (-1.09%) 6 <1> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 55.41% <100%> (-0.1%) 0 <0> (ø)
...a/com/microsoft/sqlserver/jdbc/TdsTimeoutTask.java 59.09% <59.09%> (ø) 4 <4> (?)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 40.65% <0%> (-3.3%) 13% <0%> (-2%)
...m/microsoft/sqlserver/jdbc/SQLServerException.java 76.29% <0%> (-1.49%) 31% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 45.55% <0%> (-1.09%) 110% <0%> (-2%)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 66.53% <0%> (-0.81%) 64% <0%> (ø)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 38.36% <0%> (-0.65%) 42% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 68.35% <0%> (-0.24%) 0% <0%> (ø)
... and 5 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 fd2f3d3...70e3cf1. Read the comment docs.

1 similar comment
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2 into shared-timer-for-timeouts will decrease coverage by 0.11%.
The diff coverage is 66.66%.

Impacted file tree graph

@@                       Coverage Diff                       @@
##             shared-timer-for-timeouts       #2      +/-   ##
===============================================================
- Coverage                        50.37%   50.26%   -0.12%     
+ Complexity                        2931     2921      -10     
===============================================================
  Files                              122      120       -2     
  Lines                            28108    28098      -10     
  Branches                          4692     4694       +2     
===============================================================
- Hits                             14160    14124      -36     
- Misses                           11643    11679      +36     
+ Partials                          2305     2295      -10
Flag Coverage Δ Complexity Δ
#JDBC42 49.85% <66.66%> (-0.12%) 2882 <5> (-12)
#JDBC43 50.22% <66.66%> (-0.07%) 2919 <5> (-5)
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 51.47% <100%> (ø) 255 <0> (ø) ⬇️
...java/com/microsoft/sqlserver/jdbc/SharedTimer.java 73.91% <100%> (-1.09%) 6 <1> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 55.41% <100%> (-0.1%) 0 <0> (ø)
...a/com/microsoft/sqlserver/jdbc/TdsTimeoutTask.java 59.09% <59.09%> (ø) 4 <4> (?)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 40.65% <0%> (-3.3%) 13% <0%> (-2%)
...m/microsoft/sqlserver/jdbc/SQLServerException.java 76.29% <0%> (-1.49%) 31% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 45.55% <0%> (-1.09%) 110% <0%> (-2%)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 66.53% <0%> (-0.81%) 64% <0%> (ø)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 38.36% <0%> (-0.65%) 42% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 68.35% <0%> (-0.24%) 0% <0%> (ø)
... and 5 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 fd2f3d3...70e3cf1. Read the comment docs.

@sehrope sehrope force-pushed the shared-timer-for-timeouts branch from fd2f3d3 to e76e819 Compare January 10, 2019 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants