bookkeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zhai...@apache.org
Subject [bookkeeper] branch master updated: ISSUE #296: Bookie supports ephemeral port
Date Fri, 28 Jul 2017 03:11:29 GMT
This is an automated email from the ASF dual-hosted git repository.

zhaijia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 7a480d3  ISSUE #296: Bookie supports ephemeral port
7a480d3 is described below

commit 7a480d3f8e5ec5c8c3592c332cbcab19b4043dad
Author: zhaijack <zhaijia03@gmail.com>
AuthorDate: Fri Jul 28 11:11:17 2017 +0800

    ISSUE #296: Bookie supports ephemeral port
    
    Descriptions of the changes in this PR:
    
    - add a flag to allow/disable using ephemeral ports
    - change the initialization sequence to support ephmeral port
    - added two tests on verifying this behavior
    
    Author: zhaijack <zhaijia03@gmail.com>
    Author: Sijie Guo <sijie@apache.org>
    
    Reviewers: Enrico Olivelli <None>
    
    This closes #297 from zhaijack/issue_296, closes #296
---
 .../bookkeeper/conf/ServerConfiguration.java       | 29 ++++++++++++
 .../apache/bookkeeper/proto/BookieNettyServer.java | 17 +++++--
 .../org/apache/bookkeeper/proto/BookieServer.java  |  3 +-
 .../bookie/BookieInitializationTest.java           | 36 ++++++++++++++-
 .../bookkeeper/conf/TestServerConfiguration.java   | 52 ++++++++++++++++++++++
 5 files changed, 132 insertions(+), 5 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index c6f7c70..17ff7ee 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -82,6 +82,7 @@ public class ServerConfiguration extends AbstractConfiguration {
     protected final static String BOOKIE_PORT = "bookiePort";
     protected final static String LISTENING_INTERFACE = "listeningInterface";
     protected final static String ALLOW_LOOPBACK = "allowLoopback";
+    protected final static String ALLOW_EPHEMERAL_PORTS = "allowEphemeralPorts";
 
     protected final static String JOURNAL_DIR = "journalDirectory";
     protected final static String JOURNAL_DIRS = "journalDirectories";
@@ -570,6 +571,31 @@ public class ServerConfiguration extends AbstractConfiguration {
     }
 
     /**
+     * Is the bookie allowed to use an ephemeral port (port 0) as its server port.
+     *
+     * <p>By default, an ephemeral port is not allowed. Using an ephemeral port
+     * as the service port usually indicates a configuration error. However, in unit
+     * tests, using ephemeral port will address port conflicts problem and allow
+     * running tests in parallel.
+     *
+     * @return whether is allowed to use an ephemeral port.
+     */
+    public boolean getAllowEphemeralPorts() {
+        return this.getBoolean(ALLOW_EPHEMERAL_PORTS, false);
+    }
+
+    /**
+     * Configure the bookie to allow using an ephemeral port.
+     *
+     * @param allow whether to allow using an ephemeral port.
+     * @return server configuration
+     */
+    public ServerConfiguration setAllowEphemeralPorts(boolean allow) {
+        this.setProperty(ALLOW_EPHEMERAL_PORTS, allow);
+        return this;
+    }
+
+    /**
      * Return whether we should allow addition of ledger/index dirs to an existing bookie.
      *
      * @return true if the addition is allowed; false otherwise
@@ -1932,6 +1958,9 @@ public class ServerConfiguration extends AbstractConfiguration {
             throw new ConfigurationException("Entry log file size should not be larger than
"
                     + BookKeeperConstants.MAX_LOG_SIZE_LIMIT);
         }
+        if (0 == getBookiePort() && !getAllowEphemeralPorts()) {
+            throw new ConfigurationException("Invalid port specified, using ephemeral ports
accidentally?");
+        }
     }
 
     /**
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieNettyServer.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieNettyServer.java
index c1114bb..f725dca 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieNettyServer.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieNettyServer.java
@@ -83,7 +83,7 @@ class BookieNettyServer {
     final ServerConfiguration conf;
     final EventLoopGroup eventLoopGroup;
     final EventLoopGroup jvmEventLoopGroup;
-    final RequestProcessor requestProcessor;
+    RequestProcessor requestProcessor;
     final AtomicBoolean isRunning = new AtomicBoolean(false);
     final AtomicBoolean isClosed = new AtomicBoolean(false);
     final Object suspensionLock = new Object();
@@ -140,6 +140,11 @@ class BookieNettyServer {
         listenOn(bindAddress, bookieAddress);
     }
 
+    public BookieNettyServer setRequestProcessor(RequestProcessor processor) {
+        this.requestProcessor = processor;
+        return this;
+    }
+
     boolean isRunning() {
         return isRunning.get();
     }
@@ -175,7 +180,8 @@ class BookieNettyServer {
         }
     }
 
-    private void listenOn(InetSocketAddress address, BookieSocketAddress bookieAddress) throws
InterruptedException {
+    private void listenOn(InetSocketAddress address, BookieSocketAddress bookieAddress)
+            throws InterruptedException {
         if (!conf.isDisableServerSocketBind()) {
             ServerBootstrap bootstrap = new ServerBootstrap();
             bootstrap.childOption(ChannelOption.ALLOCATOR, new PooledByteBufAllocator(true));
@@ -219,7 +225,12 @@ class BookieNettyServer {
             });
 
             // Bind and start to accept incoming connections
-            bootstrap.bind(address.getAddress(), address.getPort()).sync();
+            Channel listen = bootstrap.bind(address.getAddress(), address.getPort()).sync().channel();
+            if (listen.localAddress() instanceof InetSocketAddress) {
+                if (conf.getBookiePort() == 0) {
+                    conf.setBookiePort(((InetSocketAddress) listen.localAddress()).getPort());
+                }
+            }
         }
 
         if (conf.isEnableLocalTransport()) {
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
index 42a2bb1..8335055 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
@@ -94,10 +94,11 @@ public class BookieServer {
             BookieException, UnavailableException, CompatibilityException {
         this.conf = conf;
         this.statsLogger = statsLogger;
+        this.nettyServer = new BookieNettyServer(this.conf, null);
         this.bookie = newBookie(conf);
         this.requestProcessor = new BookieRequestProcessor(conf, bookie,
                 statsLogger.scope(SERVER_SCOPE));
-        this.nettyServer = new BookieNettyServer(this.conf, requestProcessor);
+        this.nettyServer.setRequestProcessor(this.requestProcessor);
         isAutoRecoveryDaemonEnabled = conf.isAutoRecoveryDaemonEnabled();
         if (isAutoRecoveryDaemonEnabled) {
             this.autoRecoveryMain = new AutoRecoveryMain(conf, statsLogger.scope(REPLICATION_SCOPE));
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
index 34681fa..0cbebd4 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
@@ -20,6 +20,8 @@
  */
 package org.apache.bookkeeper.bookie;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -280,6 +282,38 @@ public class BookieInitializationTest extends BookKeeperClusterTestCase
{
     }
 
     /**
+     * Verify bookie server starts up on ephemeral ports.
+     */
+    @Test(timeout = 20000)
+    public void testBookieServerStartupOnEphemeralPorts() throws Exception {
+        File tmpDir = createTempDir("bookie", "test");
+
+        ServerConfiguration conf = TestBKConfiguration.newServerConfiguration();
+        conf.setZkServers(null)
+            .setBookiePort(0)
+            .setJournalDirName(tmpDir.getPath())
+            .setLedgerDirNames(
+                new String[] { tmpDir.getPath() });
+        assertEquals(0, conf.getBookiePort());
+
+        ServerConfiguration conf1 = new ServerConfiguration();
+        conf1.addConfiguration(conf);
+        BookieServer bs1 = new BookieServer(conf1);
+        bs1.start();
+        assertFalse(0 == conf1.getBookiePort());
+
+        // starting bk server with same conf
+        ServerConfiguration conf2 = new ServerConfiguration();
+        conf2.addConfiguration(conf);
+        BookieServer bs2 = new BookieServer(conf2);
+        bs2.start();
+        assertFalse(0 == conf2.getBookiePort());
+
+        // these two bookies are listening on different ports eventually
+        assertFalse(conf1.getBookiePort() == conf2.getBookiePort());
+    }
+
+    /**
      * Verify bookie start behaviour when ZK Server is not running.
      */
     @Test(timeout = 20000)
@@ -478,7 +512,7 @@ public class BookieInitializationTest extends BookKeeperClusterTestCase
{
         // minUsableSizeForIndexFileCreation to very high value, it wouldn't. be
         // able to find any index dir when all discs are full
         server.start();
-        Assert.assertFalse("Bookie should be Shutdown", server.getBookie().isRunning());
+        assertFalse("Bookie should be Shutdown", server.getBookie().isRunning());
         server.shutdown();
 
         // Here we are setting MinUsableSizeForIndexFileCreation to very low
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
new file mode 100644
index 0000000..1f62258
--- /dev/null
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
@@ -0,0 +1,52 @@
+/*
+ *
+ * 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.conf;
+
+import static org.junit.Assert.assertTrue;
+
+import org.apache.commons.configuration.ConfigurationException;
+import org.junit.Test;
+
+/**
+ * Unit test for {@link ServerConfiguration}.
+ */
+public class TestServerConfiguration {
+
+    @Test
+    public void testEphemeralPortsAllowed() throws ConfigurationException {
+        ServerConfiguration conf = new ServerConfiguration();
+        conf.setAllowEphemeralPorts(true);
+        conf.setBookiePort(0);
+
+        conf.validate();
+        assertTrue(true);
+    }
+
+    @Test(expected = ConfigurationException.class)
+    public void testEphemeralPortsDisallowed() throws ConfigurationException {
+        ServerConfiguration conf = new ServerConfiguration();
+        conf.setAllowEphemeralPorts(false);
+        conf.setBookiePort(0);
+        conf.validate();
+    }
+
+}

-- 
To stop receiving notification emails like this one, please contact
['"commits@bookkeeper.apache.org" <commits@bookkeeper.apache.org>'].

Mime
View raw message