-
Notifications
You must be signed in to change notification settings - Fork 426
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 TimeoutPoller to have one thread for timers #842
Conversation
needs test cases
fix formatting as well
Codecov Report
@@ Coverage Diff @@
## dev #842 +/- ##
===========================================
- Coverage 48.98% 48.68% -0.3%
+ Complexity 2821 2813 -8
===========================================
Files 116 118 +2
Lines 27877 27863 -14
Branches 4650 4637 -13
===========================================
- Hits 13655 13566 -89
+ Misses 12104 12101 -3
- Partials 2118 2196 +78
Continue to review full report at Codecov.
|
ok sorry, now the formatting should be correct, eclipse kept reverting the formatter I was using on my vm..... |
Please add the license header to the new files.
|
src/test/java/com/microsoft/sqlserver/jdbc/timeouts/TimeoutTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/timeouts/TimeoutTest.java
Outdated
Show resolved
Hide resolved
@ulvii anything other changes required? please let me know |
@ulvii @cheenamalhotra anything I missed? |
Hi @shayaantx , |
@ulvii no problem, just let me know, thanks |
src/test/java/com/microsoft/sqlserver/jdbc/timeouts/TimeoutTest.java
Outdated
Show resolved
Hide resolved
Other than that one comment I made, this PR looks good to me. |
…ng timeouts and infinite polling checking for timeouts Specifically in microsoft#842, I introduced a single thread to replace the infinite timeout thread pool. However the thread lives forever, so this change seeks to fix that. Also the interrupt logic before/after my change can block timeout threads. So this change also addresses that.
As discussed in other pr, modified the TimeoutTimer to not be a unlimited thread pool, instead its one thread that lets TDS commands register/deregister with it, when a timeout is configured, and that thread will timeout the command if necessary. Included test cases.
Updated to include bulk copy functionality.
I literally formatted every single piece of code I touched, it seems much of IO buffer doesn't match the linked formatter, https://github.com/Microsoft/mssql-jdbc/blob/dev/mssql-jdbc_formatter.xml