Skip to content

Commit 63f700d

Browse files
pujaganidiemol
andauthored
[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 <diemol@gmail.com> Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
1 parent 8a20973 commit 63f700d

17 files changed

Lines changed: 395 additions & 143 deletions

File tree

java/server/src/org/openqa/selenium/grid/commands/Hub.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ protected Handlers createHandlers(Config config) {
155155
secretOptions.getRegistrationSecret());
156156
handler.addHandler(distributor);
157157

158-
Router router = new Router(tracer, clientFactory, sessions, distributor);
158+
Router router = new Router(tracer, clientFactory, sessions, queuer, distributor);
159159
GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
160160
HttpHandler readinessCheck = req -> {
161161
boolean ready = router.isReady() && bus.isReady();

java/server/src/org/openqa/selenium/grid/commands/Standalone.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ protected Handlers createHandlers(Config config) {
161161
registrationSecret);
162162
combinedHandler.addHandler(distributor);
163163

164-
Routable router = new Router(tracer, clientFactory, sessions, distributor)
164+
Routable router = new Router(tracer, clientFactory, sessions, queuer, distributor)
165165
.with(networkOptions.getSpecComplianceChecks());
166166

167167
HttpHandler readinessCheck = req -> {

java/server/src/org/openqa/selenium/grid/distributor/CreateSession.java

Lines changed: 0 additions & 42 deletions
This file was deleted.

java/server/src/org/openqa/selenium/grid/distributor/Distributor.java

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,6 @@ protected Distributor(
142142

143143
Json json = new Json();
144144
routes = Route.combine(
145-
post("/session").to(() -> req -> {
146-
CreateSessionResponse sessionResponse = newSession(req);
147-
return new HttpResponse().setContent(bytes(sessionResponse.getDownstreamEncodedResponse()));
148-
}),
149-
post("/se/grid/distributor/session")
150-
.to(() -> new CreateSession(this))
151-
.with(requiresSecret),
152145
post("/se/grid/distributor/node")
153146
.to(() -> new AddNode(tracer, this, json, httpClientFactory, registrationSecret))
154147
.with(requiresSecret),
@@ -163,21 +156,7 @@ protected Distributor(
163156
.with(new SpanDecorator(tracer, req -> "distributor.status")));
164157
}
165158

166-
public CreateSessionResponse newSession(HttpRequest request) {
167-
Either<SessionNotCreatedException, CreateSessionResponse> sessionResponse =
168-
createNewSessionResponse(request);
169-
if (sessionResponse.isRight()) {
170-
return sessionResponse.right();
171-
} else {
172-
SessionNotCreatedException exception = sessionResponse.left();
173-
if (exception instanceof RetrySessionRequestException) {
174-
throw new SessionNotCreatedException(exception.getMessage(), exception);
175-
}
176-
throw sessionResponse.left();
177-
}
178-
}
179-
180-
public Either<SessionNotCreatedException, CreateSessionResponse> createNewSessionResponse(
159+
public Either<SessionNotCreatedException, CreateSessionResponse> newSession(
181160
HttpRequest request) throws SessionNotCreatedException {
182161

183162
Span span = newSpanAsChildOf(tracer, request, "distributor.create_session_response");

java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ private void handleNewSessionRequest(HttpRequest sessionRequest, RequestId reqId
150150

151151
attributeMap.put("request", EventAttribute.setValue(sessionRequest.toString()));
152152
Either<SessionNotCreatedException, CreateSessionResponse> response =
153-
createNewSessionResponse(sessionRequest);
153+
newSession(sessionRequest);
154154
if (response.isRight()) {
155155
CreateSessionResponse sessionResponse = response.right();
156156
NewSessionResponse newSessionResponse =

java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,6 @@ public boolean isReady() {
7676
}
7777
}
7878

79-
@Override
80-
public CreateSessionResponse newSession(HttpRequest request)
81-
throws SessionNotCreatedException {
82-
HttpRequest upstream = new HttpRequest(POST, "/se/grid/distributor/session");
83-
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
84-
upstream.setContent(request.getContent());
85-
86-
HttpResponse response = client.with(addSecret).execute(upstream);
87-
88-
return Values.get(response, CreateSessionResponse.class);
89-
}
90-
9179
@Override
9280
public RemoteDistributor add(Node node) {
9381
HttpRequest request = new HttpRequest(POST, "/se/grid/distributor/node");

java/server/src/org/openqa/selenium/grid/router/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ java_library(
1919
"//java/server/src/org/openqa/selenium/grid/data",
2020
"//java/server/src/org/openqa/selenium/grid/distributor",
2121
"//java/server/src/org/openqa/selenium/grid/sessionmap",
22+
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
2223
"//java/server/src/org/openqa/selenium/grid/web",
2324
"//java/server/src/org/openqa/selenium/status",
2425
artifact("com.google.guava:guava"),

java/server/src/org/openqa/selenium/grid/router/Router.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.common.collect.ImmutableSet;
2121
import org.openqa.selenium.grid.distributor.Distributor;
2222
import org.openqa.selenium.grid.sessionmap.SessionMap;
23+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
2324
import org.openqa.selenium.internal.Require;
2425
import org.openqa.selenium.json.Json;
2526
import org.openqa.selenium.remote.http.HttpClient;
@@ -42,23 +43,27 @@ public class Router implements HasReadyState, Routable {
4243
private final Routable routes;
4344
private final SessionMap sessions;
4445
private final Distributor distributor;
46+
private final NewSessionQueuer queuer;
4547

4648
public Router(
4749
Tracer tracer,
4850
HttpClient.Factory clientFactory,
4951
SessionMap sessions,
52+
NewSessionQueuer queuer,
5053
Distributor distributor) {
5154
Require.nonNull("Tracer to use", tracer);
5255
Require.nonNull("HTTP client factory", clientFactory);
5356

5457
this.sessions = Require.nonNull("Session map", sessions);
58+
this.queuer = Require.nonNull("New Session Request Queuer", queuer);
5559
this.distributor = Require.nonNull("Distributor", distributor);
5660

5761
routes =
5862
combine(
5963
get("/status")
6064
.to(() -> new GridStatusHandler(new Json(), tracer, clientFactory, distributor)),
6165
sessions.with(new SpanDecorator(tracer, req -> "session_map")),
66+
queuer.with(new SpanDecorator(tracer, req -> "session_queuer")),
6267
distributor.with(new SpanDecorator(tracer, req -> "distributor")),
6368
matching(req -> req.getUri().startsWith("/session/"))
6469
.to(() -> new HandleSession(tracer, clientFactory, sessions)));
@@ -67,7 +72,7 @@ public Router(
6772
@Override
6873
public boolean isReady() {
6974
try {
70-
return ImmutableSet.of(distributor, sessions).parallelStream()
75+
return ImmutableSet.of(distributor, sessions, queuer).parallelStream()
7176
.map(HasReadyState::isReady)
7277
.reduce(true, Boolean::logicalAnd);
7378
} catch (RuntimeException e) {

java/server/src/org/openqa/selenium/grid/router/httpd/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ java_library(
2525
"//java/server/src/org/openqa/selenium/grid/server",
2626
"//java/server/src/org/openqa/selenium/grid/sessionmap",
2727
"//java/server/src/org/openqa/selenium/grid/sessionmap/config",
28+
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
29+
"//java/server/src/org/openqa/selenium/grid/sessionqueue/config",
30+
"//java/server/src/org/openqa/selenium/grid/sessionqueue/remote",
2831
"//java/server/src/org/openqa/selenium/netty/server",
2932
artifact("com.beust:jcommander"),
3033
artifact("com.google.guava:guava"),

java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@
3939
import org.openqa.selenium.grid.server.Server;
4040
import org.openqa.selenium.grid.sessionmap.SessionMap;
4141
import org.openqa.selenium.grid.sessionmap.config.SessionMapOptions;
42+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
43+
import org.openqa.selenium.grid.sessionqueue.config.NewSessionQueuerOptions;
44+
import org.openqa.selenium.grid.sessionqueue.remote.RemoteNewSessionQueuer;
4245
import org.openqa.selenium.internal.Require;
4346
import org.openqa.selenium.remote.http.HttpClient;
4447
import org.openqa.selenium.remote.http.HttpResponse;
@@ -55,6 +58,7 @@
5558
import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
5659
import static org.openqa.selenium.grid.config.StandardGridRoles.ROUTER_ROLE;
5760
import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_MAP_ROLE;
61+
import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUER_ROLE;
5862
import static org.openqa.selenium.net.Urls.fromUri;
5963
import static org.openqa.selenium.remote.http.Route.get;
6064

@@ -75,7 +79,11 @@ public String getDescription() {
7579

7680
@Override
7781
public Set<Role> getConfigurableRoles() {
78-
return ImmutableSet.of(DISTRIBUTOR_ROLE, HTTPD_ROLE, ROUTER_ROLE, SESSION_MAP_ROLE);
82+
return ImmutableSet.of(
83+
DISTRIBUTOR_ROLE,
84+
HTTPD_ROLE, ROUTER_ROLE,
85+
SESSION_MAP_ROLE,
86+
SESSION_QUEUER_ROLE);
7987
}
8088

8189
@Override
@@ -104,6 +112,12 @@ protected Handlers createHandlers(Config config) {
104112
SessionMapOptions sessionsOptions = new SessionMapOptions(config);
105113
SessionMap sessions = sessionsOptions.getSessionMap();
106114

115+
NewSessionQueuerOptions sessionQueuerOptions = new NewSessionQueuerOptions(config);
116+
URL sessionQueuerUrl = fromUri(sessionQueuerOptions.getSessionQueuerUri());
117+
NewSessionQueuer queuer = new RemoteNewSessionQueuer(
118+
tracer,
119+
clientFactory.createClient(sessionQueuerUrl));
120+
107121
BaseServerOptions serverOptions = new BaseServerOptions(config);
108122
SecretOptions secretOptions = new SecretOptions(config);
109123

@@ -118,7 +132,7 @@ protected Handlers createHandlers(Config config) {
118132
GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
119133

120134
Route handler = Route.combine(
121-
new Router(tracer, clientFactory, sessions, distributor).with(networkOptions.getSpecComplianceChecks()),
135+
new Router(tracer, clientFactory, sessions, queuer, distributor).with(networkOptions.getSpecComplianceChecks()),
122136
Route.post("/graphql").to(() -> graphqlHandler),
123137
get("/readyz").to(() -> req -> new HttpResponse().setStatus(HTTP_NO_CONTENT)));
124138

0 commit comments

Comments
 (0)