Use Socket instead of SocketChannel when multiSubnetFailover=true#662
Use Socket instead of SocketChannel when multiSubnetFailover=true#662ulvii merged 1 commit intomicrosoft:devfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #662 +/- ##
============================================
+ Coverage 48.08% 48.37% +0.28%
- Complexity 2575 2593 +18
============================================
Files 113 113
Lines 26574 26574
Branches 4429 4430 +1
============================================
+ Hits 12778 12854 +76
- Misses 11665 11674 +9
+ Partials 2131 2046 -85
Continue to review full report at Codecov.
|
|
Hi @greg-pendlebury and @sehrope, We would very much appreciate if you could test the attached jars against #144 and #645. |
|
Thanks @ulvii, one of our DBAs tested the jar with JMeter using the same test we used in demonstrating #645. He has confirmed that the NUMA affinity issues are resolved. We will try updating our full application once a release of the driver is available, but the regression testing of all the other changes since 6.1.7 might take us some time. |
|
Sure we'll get this tested. Are the jars you've attached built off the tip of this PR? i.e. can I just build the PR? I took a peek at the diff itself and it's quite minimal. The main change seems to be replacing |
|
Thanks @greg-pendlebury , @sehrope . @sehrope , yes the jars are built on top of the PR. |
|
We tested this a bit and it doesn't break query cancellation when multiSubnetFailover is enabled. I think it'd be worthwhile to eventually rewrite how The current code also handles IPv4 and IPv6 sequentially. I don't see that behavior explicitly documented anywhere nor does it seem like it'd be useful. If a user has multiple address types resolving from a host name they should all be attempted in parallel right? I took a stab at a high level clean up for this. See here for a PR: #663 |
* Added more information to error messages To help debug an irreproducable/random mismatch error if it occurs in the future. * Fix for the issue when using setMaxRows() with SHOWPLAN ON (#666) * Dont throw exception for colmetadata token * Adding a comment * Update comment * Adding a warning message * remove ignoreLengthPrefixedToken * Fix for uncaught/unhandled exception (#664) * Added more information to error messages To help debug an irreproducable/random mismatch error if it occurs in the future. * Revert "Added information to error message" This reverts commit 25301e6. * Fix for #659 Added error handling logic for special cases. * Read message length Read the message length instead of reading until terminating character * Unsigned byte update Message length is an unsigned byte, converting before using. * Removed clunky hex conversions convert the byte straight to an int and use existing constants instead of making new ones * Narrowed trigger conditions fixed an issue where column names who had the hex token 'AA' would cause an error to be thrown. * Spacing fixes * Added test case * spacing adjustment * Edited test drop procedures Changed IF EXISTS DROP commands to be compatible with sql server 2008 * github spacing misalignment fixes * Changed test condition now only runs on compatible database or higher * Removed error check Removed a previous implementation in favor of one that changes the TDS parser * tdsreader change * removing test for now * enabled tests * github spacing fix * removed array import * removed "arrays" instead of "array" * spacing changes * Use Socket instead of SocketChannel when multiSubnetFailover=true (#662) * Upped SQL Server requirement to 2017 * Removing Exception Test Implement a more generic and compatible test in the future * Removed imports Used in removed test
* Added more information to error messages To help debug an irreproducable/random mismatch error if it occurs in the future. * Fix for the issue when using setMaxRows() with SHOWPLAN ON (#666) * Dont throw exception for colmetadata token * Adding a comment * Update comment * Adding a warning message * remove ignoreLengthPrefixedToken * Fix for uncaught/unhandled exception (#664) * Added more information to error messages To help debug an irreproducable/random mismatch error if it occurs in the future. * Revert "Added information to error message" This reverts commit 25301e6. * Fix for #659 Added error handling logic for special cases. * Read message length Read the message length instead of reading until terminating character * Unsigned byte update Message length is an unsigned byte, converting before using. * Removed clunky hex conversions convert the byte straight to an int and use existing constants instead of making new ones * Narrowed trigger conditions fixed an issue where column names who had the hex token 'AA' would cause an error to be thrown. * Spacing fixes * Added test case * spacing adjustment * Edited test drop procedures Changed IF EXISTS DROP commands to be compatible with sql server 2008 * github spacing misalignment fixes * Changed test condition now only runs on compatible database or higher * Removed error check Removed a previous implementation in favor of one that changes the TDS parser * tdsreader change * removing test for now * enabled tests * github spacing fix * removed array import * removed "arrays" instead of "array" * spacing changes * Use Socket instead of SocketChannel when multiSubnetFailover=true (#662) * Upped SQL Server requirement to 2017 * Removing Exception Test Implement a more generic and compatible test in the future * Removed imports Used in removed test * Change in preperation for 6.5.1 preview release * Removed SNAPSHOT from POM * Added missing link * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md
Fix for both #144 and #645 .This PR reverts #160.
Switching from SocketChannels to Sockets to avoid creating 2 connections when multiSubnetFailover=true.