[grid] Integrated NewSessionQueuer with the Router. (#8856)

* Reverting change on Hub.java file to reduce diff [skip ci] * [grid] Integrate NewSessionQueuer with Router. Remove endpoints to create session in Distributor. * [grid] Update Distributor newSession() to return Either. Update Distributor tests. Delete CreateSession. Update Graphql test. * [grid] Undo formatting changes to Hub. * [grid] Fix continuation indentation for SessionQueueGridTest. Co-authored-by: Diego Molina <[email protected]> Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · Nov 10, 2020 · 63f700d · 63f700d
1 parent 8a20973
commit 63f700d
Show file tree

Hide file tree

Showing 17 changed files with 395 additions and 143 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Hub.java b/java/server/src/org/openqa/selenium/grid/commands/Hub.java
@@ -155,7 +155,7 @@ protected Handlers createHandlers(Config config) {
       secretOptions.getRegistrationSecret());
     handler.addHandler(distributor);
 
-    Router router = new Router(tracer, clientFactory, sessions, distributor);
+    Router router = new Router(tracer, clientFactory, sessions, queuer, distributor);
     GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
     HttpHandler readinessCheck = req -> {
       boolean ready = router.isReady() && bus.isReady();

diff --git a/java/server/src/org/openqa/selenium/grid/commands/Standalone.java b/java/server/src/org/openqa/selenium/grid/commands/Standalone.java
@@ -161,7 +161,7 @@ protected Handlers createHandlers(Config config) {
       registrationSecret);
     combinedHandler.addHandler(distributor);
 
-    Routable router = new Router(tracer, clientFactory, sessions, distributor)
+    Routable router = new Router(tracer, clientFactory, sessions, queuer, distributor)
       .with(networkOptions.getSpecComplianceChecks());
 
     HttpHandler readinessCheck = req -> {

diff --git a/java/server/src/org/openqa/selenium/grid/distributor/CreateSession.java b/java/server/src/org/openqa/selenium/grid/distributor/CreateSession.java
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java b/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java
@@ -142,13 +142,6 @@ protected Distributor(
 
     Json json = new Json();
     routes = Route.combine(
-      post("/session").to(() -> req -> {
-        CreateSessionResponse sessionResponse = newSession(req);
-        return new HttpResponse().setContent(bytes(sessionResponse.getDownstreamEncodedResponse()));
-      }),
-      post("/se/grid/distributor/session")
-          .to(() -> new CreateSession(this))
-          .with(requiresSecret),
       post("/se/grid/distributor/node")
           .to(() -> new AddNode(tracer, this, json, httpClientFactory, registrationSecret))
           .with(requiresSecret),
@@ -163,21 +156,7 @@ protected Distributor(
           .with(new SpanDecorator(tracer, req -> "distributor.status")));
   }
 
-  public CreateSessionResponse newSession(HttpRequest request) {
-    Either<SessionNotCreatedException, CreateSessionResponse> sessionResponse =
-      createNewSessionResponse(request);
-    if (sessionResponse.isRight()) {
-      return sessionResponse.right();
-    } else {
-      SessionNotCreatedException exception = sessionResponse.left();
-      if (exception instanceof RetrySessionRequestException) {
-        throw new SessionNotCreatedException(exception.getMessage(), exception);
-      }
-      throw sessionResponse.left();
-    }
-  }
-
-  public Either<SessionNotCreatedException, CreateSessionResponse> createNewSessionResponse(
+  public Either<SessionNotCreatedException, CreateSessionResponse> newSession(
     HttpRequest request) throws SessionNotCreatedException {
 
     Span span = newSpanAsChildOf(tracer, request, "distributor.create_session_response");

diff --git a/java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -150,7 +150,7 @@ private void handleNewSessionRequest(HttpRequest sessionRequest, RequestId reqId
 
       attributeMap.put("request", EventAttribute.setValue(sessionRequest.toString()));
       Either<SessionNotCreatedException, CreateSessionResponse> response =
-          createNewSessionResponse(sessionRequest);
+          newSession(sessionRequest);
       if (response.isRight()) {
         CreateSessionResponse sessionResponse = response.right();
         NewSessionResponse newSessionResponse =

diff --git a/java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java b/java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java
@@ -76,18 +76,6 @@ public boolean isReady() {
     }
   }
 
-  @Override
-  public CreateSessionResponse newSession(HttpRequest request)
-      throws SessionNotCreatedException {
-    HttpRequest upstream = new HttpRequest(POST, "/se/grid/distributor/session");
-    HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
-    upstream.setContent(request.getContent());
-
-    HttpResponse response = client.with(addSecret).execute(upstream);
-
-    return Values.get(response, CreateSessionResponse.class);
-  }
-
   @Override
   public RemoteDistributor add(Node node) {
     HttpRequest request = new HttpRequest(POST, "/se/grid/distributor/node");

diff --git a/java/server/src/org/openqa/selenium/grid/router/BUILD.bazel b/java/server/src/org/openqa/selenium/grid/router/BUILD.bazel
@@ -19,6 +19,7 @@ java_library(
         "//java/server/src/org/openqa/selenium/grid/data",
         "//java/server/src/org/openqa/selenium/grid/distributor",
         "//java/server/src/org/openqa/selenium/grid/sessionmap",
+        "//java/server/src/org/openqa/selenium/grid/sessionqueue",
         "//java/server/src/org/openqa/selenium/grid/web",
         "//java/server/src/org/openqa/selenium/status",
         artifact("com.google.guava:guava"),

diff --git a/java/server/src/org/openqa/selenium/grid/router/Router.java b/java/server/src/org/openqa/selenium/grid/router/Router.java
@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableSet;
 import org.openqa.selenium.grid.distributor.Distributor;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
+import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.http.HttpClient;
@@ -42,23 +43,27 @@ public class Router implements HasReadyState, Routable {
   private final Routable routes;
   private final SessionMap sessions;
   private final Distributor distributor;
+  private final NewSessionQueuer queuer;
 
   public Router(
     Tracer tracer,
     HttpClient.Factory clientFactory,
     SessionMap sessions,
+    NewSessionQueuer queuer,
     Distributor distributor) {
     Require.nonNull("Tracer to use", tracer);
     Require.nonNull("HTTP client factory", clientFactory);
 
     this.sessions = Require.nonNull("Session map", sessions);
+    this.queuer = Require.nonNull("New Session Request Queuer", queuer);
     this.distributor = Require.nonNull("Distributor", distributor);
 
     routes =
       combine(
         get("/status")
           .to(() -> new GridStatusHandler(new Json(), tracer, clientFactory, distributor)),
         sessions.with(new SpanDecorator(tracer, req -> "session_map")),
+        queuer.with(new SpanDecorator(tracer, req -> "session_queuer")),
         distributor.with(new SpanDecorator(tracer, req -> "distributor")),
         matching(req -> req.getUri().startsWith("/session/"))
           .to(() -> new HandleSession(tracer, clientFactory, sessions)));
@@ -67,7 +72,7 @@ public Router(
   @Override
   public boolean isReady() {
     try {
-      return ImmutableSet.of(distributor, sessions).parallelStream()
+      return ImmutableSet.of(distributor, sessions, queuer).parallelStream()
         .map(HasReadyState::isReady)
         .reduce(true, Boolean::logicalAnd);
     } catch (RuntimeException e) {

diff --git a/java/server/src/org/openqa/selenium/grid/router/httpd/BUILD.bazel b/java/server/src/org/openqa/selenium/grid/router/httpd/BUILD.bazel
@@ -25,6 +25,9 @@ java_library(
         "//java/server/src/org/openqa/selenium/grid/server",
         "//java/server/src/org/openqa/selenium/grid/sessionmap",
         "//java/server/src/org/openqa/selenium/grid/sessionmap/config",
+        "//java/server/src/org/openqa/selenium/grid/sessionqueue",
+        "//java/server/src/org/openqa/selenium/grid/sessionqueue/config",
+        "//java/server/src/org/openqa/selenium/grid/sessionqueue/remote",
         "//java/server/src/org/openqa/selenium/netty/server",
         artifact("com.beust:jcommander"),
         artifact("com.google.guava:guava"),

diff --git a/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java b/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java
@@ -39,6 +39,9 @@
 import org.openqa.selenium.grid.server.Server;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.config.SessionMapOptions;
+import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
+import org.openqa.selenium.grid.sessionqueue.config.NewSessionQueuerOptions;
+import org.openqa.selenium.grid.sessionqueue.remote.RemoteNewSessionQueuer;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.http.HttpClient;
 import org.openqa.selenium.remote.http.HttpResponse;
@@ -55,6 +58,7 @@
 import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
 import static org.openqa.selenium.grid.config.StandardGridRoles.ROUTER_ROLE;
 import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_MAP_ROLE;
+import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUER_ROLE;
 import static org.openqa.selenium.net.Urls.fromUri;
 import static org.openqa.selenium.remote.http.Route.get;
 
@@ -75,7 +79,11 @@ public String getDescription() {
 
   @Override
   public Set<Role> getConfigurableRoles() {
-    return ImmutableSet.of(DISTRIBUTOR_ROLE, HTTPD_ROLE, ROUTER_ROLE, SESSION_MAP_ROLE);
+    return ImmutableSet.of(
+        DISTRIBUTOR_ROLE,
+        HTTPD_ROLE, ROUTER_ROLE,
+        SESSION_MAP_ROLE,
+        SESSION_QUEUER_ROLE);
   }
 
   @Override
@@ -104,6 +112,12 @@ protected Handlers createHandlers(Config config) {
     SessionMapOptions sessionsOptions = new SessionMapOptions(config);
     SessionMap sessions = sessionsOptions.getSessionMap();
 
+    NewSessionQueuerOptions sessionQueuerOptions = new NewSessionQueuerOptions(config);
+    URL sessionQueuerUrl = fromUri(sessionQueuerOptions.getSessionQueuerUri());
+    NewSessionQueuer queuer = new RemoteNewSessionQueuer(
+        tracer,
+        clientFactory.createClient(sessionQueuerUrl));
+
     BaseServerOptions serverOptions = new BaseServerOptions(config);
     SecretOptions secretOptions = new SecretOptions(config);
 
@@ -118,7 +132,7 @@ protected Handlers createHandlers(Config config) {
     GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
 
     Route handler = Route.combine(
-      new Router(tracer, clientFactory, sessions, distributor).with(networkOptions.getSpecComplianceChecks()),
+      new Router(tracer, clientFactory, sessions, queuer, distributor).with(networkOptions.getSpecComplianceChecks()),
       Route.post("/graphql").to(() -> graphqlHandler),
       get("/readyz").to(() -> req -> new HttpResponse().setStatus(HTTP_NO_CONTENT)));