Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/451#discussion_r166671671
--- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
---
@@ -58,48 +61,122 @@
public StaticHostProvider(Collection<InetSocketAddress>
serverAddresses) {
for (InetSocketAddress address : serverAddresses) {
try {
- InetAddress ia = address.getAddress();
- InetAddress resolvedAddresses[] =
InetAddress.getAllByName((ia != null) ? ia.getHostAddress() :
- address.getHostName());
+ InetAddress resolvedAddresses[] =
InetAddress.getAllByName(getHostString(address));
for (InetAddress resolvedAddress : resolvedAddresses) {
- // If hostName is null but the address is not, we can
tell that
- // the hostName is an literal IP address. Then we can
set the host string as the hostname
- // safely to avoid reverse DNS lookup.
- // As far as i know, the only way to check if the
hostName is null is use toString().
- // Both the two implementations of InetAddress are
final class, so we can trust the return value of
- // the toString() method.
- if (resolvedAddress.toString().startsWith("/")
- && resolvedAddress.getAddress() != null) {
- this.serverAddresses.add(
- new
InetSocketAddress(InetAddress.getByAddress(
- address.getHostName(),
- resolvedAddress.getAddress()),
- address.getPort()));
- } else {
- this.serverAddresses.add(new
InetSocketAddress(resolvedAddress.getHostAddress(), address.getPort()));
- }
+ this.serverAddresses.add(new
InetSocketAddress(resolvedAddress, address.getPort()));
}
} catch (UnknownHostException e) {
LOG.error("Unable to connect to server: {}", address, e);
}
}
-
+
if (this.serverAddresses.isEmpty()) {
throw new IllegalArgumentException(
"A HostProvider may not be empty!");
}
Collections.shuffle(this.serverAddresses);
}
+ /**
+ * Evaluate to a hostname if one is available and otherwise it returns
the
+ * string representation of the IP address.
+ *
+ * In Java 7, we have a method getHostString, but earlier versions do
not support it.
+ * This method is to provide a replacement for
InetSocketAddress.getHostString().
+ *
+ * @param addr
+ * @return Hostname string of address parameter
+ */
+ private String getHostString(InetSocketAddress addr) {
+ String hostString = "";
+
+ if (addr == null) {
+ return hostString;
+ }
+ if (!addr.isUnresolved()) {
+ InetAddress ia = addr.getAddress();
+
+ // If the string starts with '/', then it has no hostname
+ // and we want to avoid the reverse lookup, so we return
+ // the string representation of the address.
+ if (ia.toString().startsWith("/")) {
+ hostString = ia.getHostAddress();
+ } else {
+ hostString = addr.getHostName();
+ }
+ } else {
+ // According to the Java 6 documentation, if the hostname is
+ // unresolved, then the string before the colon is the
hostname.
+ String addrString = addr.toString();
+ hostString = addrString.substring(0,
addrString.lastIndexOf(':'));
+ }
+
+ return hostString;
+ }
+
public int size() {
return serverAddresses.size();
}
+ // Counts the number of addresses added and removed during
+ // the last call to next. Used mainly for test purposes.
+ // See StasticHostProviderTest.
+ private int nextAdded = 0;
+ private int nextRemoved = 0;
+
+ public int getNextAdded() {
+ return nextAdded;
+ }
+
+ public int getNextRemoved() {
+ return nextRemoved;
+ }
+
public InetSocketAddress next(long spinDelay) {
- ++currentIndex;
- if (currentIndex == serverAddresses.size()) {
- currentIndex = 0;
+ // Handle possible connection error by re-resolving hostname if
possible
+ if (!connectedSinceNext) {
+ InetSocketAddress curAddr = serverAddresses.get(currentIndex);
+ String curHostString = getHostString(curAddr);
+ if
(!curHostString.equals(curAddr.getAddress().getHostAddress())) {
+ LOG.info("Resolving again hostname: {}",
getHostString(curAddr));
+ try {
+ int thePort = curAddr.getPort();
+ InetAddress resolvedAddresses[] =
InetAddress.getAllByName(curHostString);
+ nextAdded = 0;
+ nextRemoved = 0;
+ if (resolvedAddresses.length == 1) {
--- End diff --
It might be easier to just remove the special case of
`resolvedAddresses.length == 1` and let the other part iterate over the list
and remove the affected elements.
---
