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/12/01 00:07:19 UTC

[bookkeeper] branch branch-4.8 updated: Let OS choose port for vertx test

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 3e255e8  Let OS choose port for vertx test
3e255e8 is described below

commit 3e255e82b8aa9c3d6b71ed4679c690062489a0c3
Author: Ivan Kelly <iv...@apache.org>
AuthorDate: Sat Dec 1 01:06:29 2018 +0100

    Let OS choose port for vertx test
    
    The previous implementation was searching for a open port by
    repeatedly trying to startServer on ports starting at 8080. However,
    after the first attempt fails, the Vertx instance in VertxHttpServer
    is broken and the test hangs. 8080 is a very common port to have stuff
    running on, so these hangs have happened to be repeatedly.
    
    This fix is to allow the OS to choose the port, by specifying 0 as the
    listening port and querying afterwards.
    
    Issue: #1821
    
    Reviewers: Sijie Guo <si...@apache.org>, Enrico Olivelli <eo...@gmail.com>
    
    This closes #1853 from ivankelly/vertx-8080
    
    (cherry picked from commit b39564183eb8cab5ab21752843fe65a949fc96ff)
    Signed-off-by: Sijie Guo <si...@apache.org>
---
 .../bookkeeper/http/vertx/VertxHttpServer.java     | 10 +++++++--
 .../bookkeeper/http/vertx/TestVertxHttpServer.java | 25 +++++-----------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java
index 26c07e8..15d1039 100644
--- a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java
+++ b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java
@@ -46,11 +46,16 @@ public class VertxHttpServer implements HttpServer {
     private Vertx vertx;
     private boolean isRunning;
     private HttpServiceProvider httpServiceProvider;
+    private int listeningPort = -1;
 
     public VertxHttpServer() {
         this.vertx = Vertx.vertx();
     }
 
+    int getListeningPort() {
+        return listeningPort;
+    }
+
     @Override
     public void initialize(HttpServiceProvider httpServiceProvider) {
         this.httpServiceProvider = httpServiceProvider;
@@ -58,7 +63,7 @@ public class VertxHttpServer implements HttpServer {
 
     @Override
     public boolean startServer(int port) {
-        CompletableFuture<AsyncResult> future = new CompletableFuture<>();
+        CompletableFuture<AsyncResult<io.vertx.core.http.HttpServer>> future = new CompletableFuture<>();
         VertxHttpHandlerFactory handlerFactory = new VertxHttpHandlerFactory(httpServiceProvider);
         Router router = Router.router(vertx);
         HttpRouter<VertxAbstractHandler> requestRouter = new HttpRouter<VertxAbstractHandler>(handlerFactory) {
@@ -76,9 +81,10 @@ public class VertxHttpServer implements HttpServer {
             }
         });
         try {
-            AsyncResult asyncResult = future.get();
+            AsyncResult<io.vertx.core.http.HttpServer> asyncResult = future.get();
             if (asyncResult.succeeded()) {
                 LOG.info("Vertx Http server started successfully");
+                listeningPort = asyncResult.result().actualPort();
                 isRunning = true;
                 return true;
             } else {
diff --git a/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java b/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java
index 59fcfd8..1c547d3 100644
--- a/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java
+++ b/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java
@@ -21,6 +21,7 @@
 package org.apache.bookkeeper.http.vertx;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -39,19 +40,13 @@ import org.junit.Test;
  * Unit test {@link VertxHttpServer}.
  */
 public class TestVertxHttpServer {
-
-    private int port = 8080;
-
     @Test
     public void testStartBasicHttpServer() throws Exception {
         VertxHttpServer httpServer = new VertxHttpServer();
         HttpServiceProvider httpServiceProvider = NullHttpServiceProvider.getInstance();
         httpServer.initialize(httpServiceProvider);
-        int port = getNextPort();
-        while (!httpServer.startServer(port)) {
-            httpServer.stopServer();
-            port = getNextPort();
-        }
+        assertTrue(httpServer.startServer(0));
+        int port = httpServer.getListeningPort();
         HttpResponse httpResponse = sendGet(getUrl(port, HttpRouter.HEARTBEAT));
         assertEquals(HttpServer.StatusCode.OK.getValue(), httpResponse.responseCode);
         assertEquals(HeartbeatService.HEARTBEAT.trim(), httpResponse.responseBody.trim());
@@ -63,11 +58,8 @@ public class TestVertxHttpServer {
         VertxHttpServer httpServer = new VertxHttpServer();
         HttpServiceProvider httpServiceProvider = NullHttpServiceProvider.getInstance();
         httpServer.initialize(httpServiceProvider);
-        int port = getNextPort();
-        while (!httpServer.startServer(port)) {
-            httpServer.stopServer();
-            port = getNextPort();
-        }
+        assertTrue(httpServer.startServer(0));
+        int port = httpServer.getListeningPort();
         HttpResponse httpResponse = sendGet(getUrl(port, HttpRouter.METRICS));
         assertEquals(HttpServer.StatusCode.OK.getValue(), httpResponse.responseCode);
         httpServer.stopServer();
@@ -109,11 +101,4 @@ public class TestVertxHttpServer {
             this.responseBody = responseBody;
         }
     }
-
-    private int getNextPort() throws Exception {
-        if (port > 65535) {
-            throw new Exception("No port available");
-        }
-        return port++;
-    }
 }