resolve dns to ip addresses when establishing socket connection #167
Conversation
This file contains 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
added 2 commits
April 12, 2017 14:05
When establish a socket connection, instead of using `(Uri.Host, Uri.Port)`, we will use `(IPAddress, port)` instead to avoid issues on some platform where socket cannot be established with dns addresses due to one to many mapping. If the uri given is already an ip address, then the ip address will be used directly. Otherwise, we will perform a dns lookup to resolve to ip addresses. The resolved ip addresses will be put in an array where IPv6 addresses will be placed first. Then socket client will try to establish connections with the server's ip addresses in order until connection established successfully. This change does not implement connection pool level dns resolution. The connection pool still holds a pool for each uri (could either be a dns or ip address). While this PR does not need to be modified when connection pool level dns lookup is added as the socket client will not do a dns resolve if the uri is already an ip address. Fix for neo4j#164
zhenlineo
force-pushed
the
1.2-dns-resolve
branch
from
78c4053
to
d5d08f1
Compare
lutovich
approved these changes
Apr 20, 2017
| @@ -73,6 +72,26 @@ public TcpSocketClient(EncryptionManager encryptionManager, bool keepAlive, ILog | |||
| } | |||
| } | |||
|
|
|||
| private async Task Connect(Uri uri) | |||
| { | |||
| var addresses = await uri.ResolveAsyc(); | |||
There was a problem hiding this comment.
We should probably log these addresses in debug mode and include them in the IOException
| return addresses; | ||
| } | ||
|
|
||
| private class AddressComparer : IComparer<IPAddress> |
There was a problem hiding this comment.
Could you please add some unit tests for AddressComparer?
Added tests for 1AddressComparer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When establish a socket connection, instead of using
(Uri.Host, Uri.Port), we will use(IPAddress, port)instead to avoid issues on some platform where socket cannot be established with dns addresses due to one to many mapping.If the uri given is already an ip address, then the ip address will be used directly. Otherwise, we will perform a dns lookup to resolve to ip addresses. The resolved ip addresses will be put in an array where IPv6 addresses will be placed first. Then socket client will try to establish connections with the server's ip addresses in order until connection established successfully.
This change does not implement connection pool level dns resolution. The connection pool still holds a pool for each uri (could either be a dns or ip address). While this PR does not need to be modified when connection pool level dns lookup is added as the socket client will not do a dns resolve if the uri is already an ip address.
Fix for #164
Based on #166