Add IP address preference support for TCP connection#1015
Merged
Conversation
karinazhou
commented
Mar 31, 2021
David-Engel
reviewed
Apr 1, 2021
Contributor
|
I had a talk to someone with some networking experience and they pointed out that many applications will use addresses in the order returned from dns to connect to resources and so the first ip address in a list is likely to be the most connected route. This means that seemingly arbitrary choice of the last ip of each type may have been a deliberate attempt to choose the least used route. If this is true then that should have been called out in the code with a comment. I think trying each address returned in the order that they are returned will be more compatible with dns based load balancing schemes. When you look at parallelizing the connections you may want to keep track of the original ordering and when two connections succeed choose the first using the original ordering. |
Co-authored-by: David Engel <dengel1012@gmail.com>
…qlClient into IPAddressPreference
cheenamalhotra
added
the
Public API 🆕
Wraith2
reviewed
Apr 19, 2021
johnnypham
reviewed
Apr 23, 2021
…ent/SNI/SNITcpHandle.cs
# Conflicts: # src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
added 2 commits
April 30, 2021 15:45
This reverts commit 5642e74.
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
fd6c475 to
fb778fc
Compare
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
fb778fc to
16935d4
Compare
DavoudEshtehari
changed the title
JRahnama
reviewed
May 11, 2021
Co-authored-by: Javad <v-jarahn@microsoft.com> Co-authored-by: Cheena Malhotra <v-chmalh@microsoft.com>
Wraith2
reviewed
May 11, 2021
Wraith2
reviewed
May 11, 2021
JRahnama
reviewed
May 11, 2021
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
7939ea3 to
c41b411
Compare
+ improvement
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
c41b411 to
c71143c
Compare
Co-authored-by: Cheena Malhotra <v-chmalh@microsoft.com>
Wraith2
reviewed
May 11, 2021
Comment on lines
+453
to
+454
| string sValue = (value as string); | ||
| if (sValue is not null) |
Contributor
There was a problem hiding this comment.
Can this be if (value is string sValue) ? the is operator does the type and null checks so it's known to be a non-null string inside the if block if the condition returns true.
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
1615eac to
12ff426
Compare
JRahnama
approved these changes
May 13, 2021
+ modify CancelAsyncConnections test
…eference.xml Co-authored-by: Cheena Malhotra <v-chmalh@microsoft.com>
+ Disable TNIR in connection string
DavoudEshtehari
force-pushed
the
IPAddressPreference
branch
from
5e5f4ce to
019fce0
Compare
cheenamalhotra
approved these changes
May 15, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A new connection property
IPAddressPreferenceis added to specify the IP preference by the client application when doing TCP connection. There are three valid types:IPv4First
This is the default preference. The driver will traverse IPv4 addresses first. If none of them can be connected successfully, it continues to try IPv6 addresses if there are any.
IPv6First
The driver will traverse IPv6 addresses first. If none of them can be connected successfully, it continues to try IPv4 addresses if there are any.
UsePlatformDefault
The driver will traverse all the IP addresses in their initial orders from DNS resolution.
To keep the consistency between managed SNI and native SNI, the connection logic is changed in
SNITcpHandle:Connect(). Instead of keeping the last IPv4 and IPv6 addresses, all the valid IP addresses returned from DNS resolution will be tested in a row. There will be a separate PR containing this change in parallel.NOTE: This PR requires the change in native SNI so it requires a new version of Microsoft.Data.SqlClient.SNI to be released first.
cc @johnnypham @cheenamalhotra @David-Engel @saurabh500