sijie closed pull request #1789: Cache InetSocketAddress if hostname is
IPAddress
URL: https://github.com/apache/bookkeeper/pull/1789
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/BookieSocketAddress.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/BookieSocketAddress.java
index 6d562e3a67..cd0a20cfa4 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/BookieSocketAddress.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/BookieSocketAddress.java
@@ -22,10 +22,12 @@
import static org.apache.bookkeeper.util.BookKeeperConstants.COLON;
+import com.google.common.net.InetAddresses;
import io.netty.channel.local.LocalAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
+import java.util.Optional;
/**
* This is a data wrapper class that is an InetSocketAddress, it would use the
hostname
@@ -38,11 +40,24 @@
// Member fields that make up this class.
private final String hostname;
private final int port;
+ private final Optional<InetSocketAddress> socketAddress;
// Constructor that takes in both a port.
public BookieSocketAddress(String hostname, int port) {
this.hostname = hostname;
this.port = port;
+ /*
+ * if ipaddress is used for bookieid then lets cache InetSocketAddress
+ * otherwise not cache it. If Hostname is used for bookieid, then it is
+ * ok for node to change its ipaddress. But if ipaddress is used for
+ * bookieid then it is invalid scenario if node's ipaddress changes and
+ * nodes HostName is considered static.
+ */
+ if (InetAddresses.isInetAddress(hostname)) {
+ socketAddress = Optional.of(new InetSocketAddress(hostname, port));
+ } else {
+ socketAddress = Optional.empty();
+ }
}
// Constructor from a String "serialized" version of this class.
@@ -57,8 +72,15 @@ public BookieSocketAddress(String addr) throws
UnknownHostException {
} catch (NumberFormatException nfe) {
throw new UnknownHostException(addr);
}
+ if (InetAddresses.isInetAddress(hostname)) {
+ socketAddress = Optional.of(new InetSocketAddress(hostname, port));
+ } else {
+ socketAddress = Optional.empty();
+ }
}
+
+
// Public getters
public String getHostName() {
return hostname;
@@ -70,12 +92,15 @@ public int getPort() {
// Method to return an InetSocketAddress for the regular port.
public InetSocketAddress getSocketAddress() {
- // Return each time a new instance of the InetSocketAddress because
the hostname
- // gets resolved in its constructor and then cached forever.
- // If we keep using the same InetSocketAddress instance, if bookies
are advertising
- // hostnames and the IP change, the BK client will keep forever to try
to connect
- // to the old IP.
- return new InetSocketAddress(hostname, port);
+ /*
+ * Return each time a new instance of the InetSocketAddress if hostname
+ * is used as bookieid. If we keep using the same InetSocketAddress
+ * instance, if bookies are advertising hostnames and the IP change,
the
+ * BK client will keep forever to try to connect to the old IP.
+ */
+ return socketAddress.orElseGet(() -> {
+ return new InetSocketAddress(hostname, port);
+ });
}
/**
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/BookieSocketAddressTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/BookieSocketAddressTest.java
new file mode 100644
index 0000000000..1da5643d99
--- /dev/null
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/BookieSocketAddressTest.java
@@ -0,0 +1,50 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.net;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.net.InetSocketAddress;
+import org.junit.Test;
+
+/**
+ * Tests for BookieSocketAddress getSocketAddress cache logic.
+ */
+
+public class BookieSocketAddressTest {
+
+ @Test
+ public void testHostnameBookieId() throws Exception {
+ BookieSocketAddress hostnameAddress = new
BookieSocketAddress("localhost", 3181);
+ InetSocketAddress inetSocketAddress1 =
hostnameAddress.getSocketAddress();
+ InetSocketAddress inetSocketAddress2 =
hostnameAddress.getSocketAddress();
+ assertFalse("InetSocketAddress should be recreated",
inetSocketAddress1 == inetSocketAddress2);
+ }
+
+ @Test
+ public void testIPAddressBookieId() throws Exception {
+ BookieSocketAddress ipAddress = new BookieSocketAddress("127.0.0.1",
3181);
+ InetSocketAddress inetSocketAddress1 = ipAddress.getSocketAddress();
+ InetSocketAddress inetSocketAddress2 = ipAddress.getSocketAddress();
+ assertTrue("InetSocketAddress should be cached", inetSocketAddress1 ==
inetSocketAddress2);
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
