You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/11/08 16:49:53 UTC

[bookkeeper] branch branch-4.8 updated: Cache InetSocketAddress if hostname is IPAddress

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch branch-4.8
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/branch-4.8 by this push:
     new 17faf2b  Cache InetSocketAddress if hostname is IPAddress
17faf2b is described below

commit 17faf2b9b0d6f60c9c6056981997eb97e45f9f03
Author: Charan Reddy Guttapalem <re...@gmail.com>
AuthorDate: Thu Nov 8 08:49:32 2018 -0800

    Cache InetSocketAddress if hostname is IPAddress
    
    Descriptions of the changes in this PR:
    
    - in BookieSocketAddress if IPAddress is hostname then it is okay
    to cache InetSocketAddress, since the canonicalhostname of the node
    dont change.
    
    Reviewers: Sijie Guo <si...@apache.org>, Enrico Olivelli <eo...@gmail.com>, Matteo Merli <mm...@apache.org>, Andrey Yegorov <None>
    
    This closes #1789 from reddycharan/bookieaddressonlywhenhostname
    
    (cherry picked from commit 102475a954eb0c00c1cc2ccf022db4b7c0bb9eea)
    Signed-off-by: Sijie Guo <si...@apache.org>
---
 .../apache/bookkeeper/net/BookieSocketAddress.java | 37 +++++++++++++---
 .../bookkeeper/net/BookieSocketAddressTest.java    | 50 ++++++++++++++++++++++
 2 files changed, 81 insertions(+), 6 deletions(-)

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 6d562e3..cd0a20c 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 @@ package org.apache.bookkeeper.net;
 
 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 @@ public class BookieSocketAddress {
     // 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 class BookieSocketAddress {
         } 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 class BookieSocketAddress {
 
     // 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 0000000..1da5643
--- /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);
+    }
+}