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
Tries all hosts resolved for DNS name #2722
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for you contribution.
This brings almost no performance loss, because java.net.InetAddress#getByName(java.lang.String)
also calls java.net.InetAddress#getAllByName(java.lang.String)
, but gets [0 ] Element, just add the overhead of Collections.shuffle(hosts)
, but it can be ignored.
public static InetAddress getByName(String host)
throws UnknownHostException {
return InetAddress.getAllByName(host)[0];
}
Some comments are detailed in the code section.
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
9e9fc5a
to
e9d6395
Compare
e9d6395
to
c9f42b1
Compare
045316e
to
7f935f5
Compare
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
7f935f5
to
933d69a
Compare
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Outdated
Show resolved
Hide resolved
94dd5cd
to
93bfc3b
Compare
93bfc3b
to
b3c752e
Compare
@sazzad16 are there any further changes required? |
@adiamzn No. We're good for now. Thanks! We'll ping you in case anything comes up. Have a nice day! |
Codecov Report
@@ Coverage Diff @@
## master #2722 +/- ##
============================================
+ Coverage 58.41% 58.42% +0.01%
- Complexity 3068 3070 +2
============================================
Files 177 177
Lines 11011 11024 +13
Branches 629 632 +3
============================================
+ Hits 6432 6441 +9
- Misses 4361 4364 +3
- Partials 218 219 +1
Continue to review full report at Codecov.
|
Problem
By default the default DNS resolver in InetAddress class may cache ip addresses after successfully connecting for the first time.
This is can be problematic if we expect to be able to connect to more than one IP address behind a DNS name.
For example multiple read replicas behind a common dns name, or a failed host that was replaced and now has a new ip.
Proposed Fixed
When creating connecting to a host using a dns name, try all addresses the DNS server was able to resolve.