Skip to content

Commit 23bc00e

Browse files
pujaganidiemol
pujagani
and
diemol
authored
[grid] Integrating NewSessionQueuer with Distributor. (#8747)
* [grid] Update LocalDistributor to use registration secret and queuer in tests. Update LocalQueuer to get request timeout from config. * [grid] Fix code style. Use anyMatch() to check for capability in NodeStatus. * [grid] Add Either class. Add Either return type to create new session in Distributor. Add RetrySessionRequestException. * Reformat Hub.java file to simplify review [skip ci] * Reverting change on Hub.java file to reduce diff [skip ci] * Reformatting files to simplify review [skip ci] * Reformatting files to simplify review [skip ci] * [grid] Update Distributor createNewSessionResponse span. Reverse if condition for optional check to make it readable. * [grid] Remove request timeout check from LocalDistributor. * [grid] Remove Session Queue role from Distributor Server. * [grid] Undo reformatting changes. * [grid] Undo formatting changes in DistributorTest. * [grid] Undo formatting for EndToEndTest. Fix DistributorTest Distributor casting. * [grid] Remove unused import from EndtoEndTest. * [grid] Add NewSessionQueuerConfig to EndtoEndTest. * [grid] Fixing some line indents [skip ci] * [grid] Simplifying expressions [skip ci] * [grid] Fixing some indents and removing unused imports [skip ci] * [grid] Removing unneeded imports [skip ci] * [grid] Using `Duration.ofSeconds` instead of `ChronoUnit` [skip ci] * [grid] Using `Duration.ofSeconds` instead of `ChronoUnit` (2) [skip ci] Co-authored-by: Diego Molina <diemol@gmail.com> Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
1 parent cb04cdf commit 23bc00e

22 files changed

Lines changed: 629 additions & 90 deletions

File tree

‎java/client/src/org/openqa/selenium/internal/Either.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.internal;
19+
20+
public class Either<A, B> {
21+
private A left = null;
22+
private B right = null;
23+
24+
private Either(A a, B b) {
25+
left = a;
26+
right = b;
27+
}
28+
29+
public static <A, B> Either<A, B> left(A a) {
30+
return new Either<A, B>(a, null);
31+
}
32+
33+
public A left() {
34+
return left;
35+
}
36+
37+
public boolean isLeft() {
38+
return left != null;
39+
}
40+
41+
public boolean isRight() {
42+
return right != null;
43+
}
44+
45+
public B right() {
46+
return right;
47+
}
48+
49+
public static <A, B> Either<A, B> right(B b) {
50+
return new Either<A, B>(null, b);
51+
}
52+
}

‎java/server/src/org/openqa/selenium/grid/commands/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ java_library(
3333
"//java/server/src/org/openqa/selenium/grid/server",
3434
"//java/server/src/org/openqa/selenium/grid/sessionmap",
3535
"//java/server/src/org/openqa/selenium/grid/sessionmap/local",
36+
"//java/server/src/org/openqa/selenium/grid/sessionqueue/local",
37+
"//java/server/src/org/openqa/selenium/grid/sessionqueue/config",
38+
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
3639
"//java/server/src/org/openqa/selenium/grid/web",
3740
"//java/server/src/org/openqa/selenium/netty/server",
3841
artifact("com.beust:jcommander"),

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@
3838
import org.openqa.selenium.grid.server.Server;
3939
import org.openqa.selenium.grid.sessionmap.SessionMap;
4040
import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
41+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
42+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
43+
import org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions;
44+
import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueue;
45+
import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueuer;
4146
import org.openqa.selenium.grid.web.ClassPathResource;
4247
import org.openqa.selenium.grid.web.CombinedHandler;
4348
import org.openqa.selenium.grid.web.NoHandler;
@@ -132,11 +137,21 @@ protected Handlers createHandlers(Config config) {
132137
handler,
133138
networkOptions.getHttpClientFactory(tracer));
134139

140+
NewSessionQueueOptions newSessionQueueOptions = new NewSessionQueueOptions(config);
141+
NewSessionQueue sessionRequests = new LocalNewSessionQueue(
142+
tracer,
143+
bus,
144+
newSessionQueueOptions.getSessionRequestTimeout(),
145+
newSessionQueueOptions.getSessionRequestRetryInterval());
146+
NewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, bus, sessionRequests);
147+
handler.addHandler(queuer);
148+
135149
Distributor distributor = new LocalDistributor(
136150
tracer,
137151
bus,
138152
clientFactory,
139153
sessions,
154+
queuer,
140155
secretOptions.getRegistrationSecret());
141156
handler.addHandler(distributor);
142157

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@
4141
import org.openqa.selenium.grid.server.Server;
4242
import org.openqa.selenium.grid.sessionmap.SessionMap;
4343
import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
44+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
45+
import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
46+
import org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions;
47+
import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueue;
48+
import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueuer;
4449
import org.openqa.selenium.grid.web.ClassPathResource;
4550
import org.openqa.selenium.grid.web.CombinedHandler;
4651
import org.openqa.selenium.grid.web.NoHandler;
@@ -69,6 +74,7 @@
6974
import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
7075
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
7176
import static org.openqa.selenium.grid.config.StandardGridRoles.ROUTER_ROLE;
77+
import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUE_ROLE;
7278
import static org.openqa.selenium.remote.http.Route.combine;
7379
import static org.openqa.selenium.remote.http.Route.get;
7480

@@ -89,7 +95,7 @@ public String getDescription() {
8995

9096
@Override
9197
public Set<Role> getConfigurableRoles() {
92-
return ImmutableSet.of(HTTPD_ROLE, NODE_ROLE, ROUTER_ROLE);
98+
return ImmutableSet.of(HTTPD_ROLE, NODE_ROLE, ROUTER_ROLE, SESSION_QUEUE_ROLE);
9399
}
94100

95101
@Override
@@ -136,7 +142,23 @@ protected Handlers createHandlers(Config config) {
136142

137143
SessionMap sessions = new LocalSessionMap(tracer, bus);
138144
combinedHandler.addHandler(sessions);
139-
Distributor distributor = new LocalDistributor(tracer, bus, clientFactory, sessions, registrationSecret);
145+
146+
NewSessionQueueOptions newSessionQueueOptions = new NewSessionQueueOptions(config);
147+
NewSessionQueue sessionRequests = new LocalNewSessionQueue(
148+
tracer,
149+
bus,
150+
newSessionQueueOptions.getSessionRequestRetryInterval(),
151+
newSessionQueueOptions.getSessionRequestTimeout());
152+
NewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, bus, sessionRequests);
153+
combinedHandler.addHandler(queuer);
154+
155+
Distributor distributor = new LocalDistributor(
156+
tracer,
157+
bus,
158+
clientFactory,
159+
sessions,
160+
queuer,
161+
registrationSecret);
140162
combinedHandler.addHandler(distributor);
141163

142164
Routable router = new Router(tracer, clientFactory, sessions, distributor)

‎java/server/src/org/openqa/selenium/grid/data/NodeStatus.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,16 @@ public NodeStatus(
5959
}
6060
}
6161

62+
public boolean hasCapability(Capabilities caps) {
63+
return slots.stream().anyMatch(slot -> slot.isSupporting(caps));
64+
}
65+
6266
public boolean hasCapacity() {
63-
return slots.stream().anyMatch(slot -> !slot.getSession().isPresent());
67+
return slots.stream().anyMatch(slot -> slot.getSession().isEmpty());
6468
}
6569

6670
public boolean hasCapacity(Capabilities caps) {
67-
long count = slots.stream()
68-
.filter(slot -> !slot.getSession().isPresent())
69-
.filter(slot -> slot.isSupporting(caps))
70-
.count();
71-
72-
return count > 0;
71+
return slots.stream().anyMatch(slot -> slot.getSession().isEmpty() && slot.isSupporting(caps));
7372
}
7473

7574
public NodeId getId() {

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

Lines changed: 58 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.openqa.selenium.grid.security.RequiresSecretFilter;
3434
import org.openqa.selenium.grid.security.Secret;
3535
import org.openqa.selenium.grid.sessionmap.SessionMap;
36+
import org.openqa.selenium.internal.Either;
3637
import org.openqa.selenium.internal.Require;
3738
import org.openqa.selenium.json.Json;
3839
import org.openqa.selenium.remote.NewSessionPayload;
@@ -162,10 +163,24 @@ protected Distributor(
162163
.with(new SpanDecorator(tracer, req -> "distributor.status")));
163164
}
164165

165-
public CreateSessionResponse newSession(HttpRequest request)
166-
throws SessionNotCreatedException {
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(
181+
HttpRequest request) throws SessionNotCreatedException {
167182

168-
Span span = newSpanAsChildOf(tracer, request, "distributor.new_session");
183+
Span span = newSpanAsChildOf(tracer, request, "distributor.create_session_response");
169184
Map<String, EventAttributeValue> attributeMap = new HashMap<>();
170185
try (
171186
Reader reader = reader(request);
@@ -183,10 +198,10 @@ public CreateSessionResponse newSession(HttpRequest request)
183198
SessionNotCreatedException exception = new SessionNotCreatedException("No capabilities found");
184199
EXCEPTION.accept(attributeMap, exception);
185200
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
186-
EventAttribute.setValue("Unable to create session. No capabilities found: "
187-
+ exception.getMessage()));
201+
EventAttribute.setValue("Unable to create session. No capabilities found: " +
202+
exception.getMessage()));
188203
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
189-
throw exception;
204+
return Either.left(exception);
190205
}
191206

192207
Optional<Supplier<CreateSessionResponse>> selected;
@@ -200,6 +215,16 @@ public CreateSessionResponse newSession(HttpRequest request)
200215
try {
201216
Set<NodeStatus> model = ImmutableSet.copyOf(getAvailableNodes());
202217

218+
// Reject new session immediately if no node has the required capabilities
219+
boolean hostsWithCaps = model.stream()
220+
.anyMatch(nodeStatus -> nodeStatus.hasCapability(firstRequest.getCapabilities()));
221+
222+
if (!hostsWithCaps) {
223+
throw new SessionNotCreatedException(
224+
"No host supports the capabilities required: " + payload.stream()
225+
.map(Capabilities::toString).collect(Collectors.joining(", ")));
226+
}
227+
203228
// Find a host that supports the capabilities present in the new session
204229
Set<SlotId> slotIds = slotSelector.selectSlot(firstRequest.getCapabilities(), model);
205230
if (!slotIds.isEmpty()) {
@@ -211,38 +236,31 @@ public CreateSessionResponse newSession(HttpRequest request)
211236
writeLock.unlock();
212237
}
213238

214-
CreateSessionResponse sessionResponse = selected
215-
.orElseThrow(
216-
() -> {
217-
span.setAttribute("error", true);
218-
SessionNotCreatedException
219-
exception =
220-
new SessionNotCreatedException(
221-
"Unable to find provider for session: " + payload.stream()
222-
.map(Capabilities::toString).collect(Collectors.joining(", ")));
223-
EXCEPTION.accept(attributeMap, exception);
224-
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
225-
EventAttribute.setValue(
226-
"Unable to find provider for session: "
227-
+ exception.getMessage()));
228-
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
229-
return exception;
230-
})
231-
.get();
232-
233-
sessions.add(sessionResponse.getSession());
234-
235-
SessionId sessionId = sessionResponse.getSession().getId();
236-
Capabilities caps = sessionResponse.getSession().getCapabilities();
237-
String sessionUri = sessionResponse.getSession().getUri().toString();
238-
SESSION_ID.accept(span, sessionId);
239-
CAPABILITIES.accept(span, caps);
240-
SESSION_ID_EVENT.accept(attributeMap, sessionId);
241-
CAPABILITIES_EVENT.accept(attributeMap, caps);
242-
span.setAttribute(AttributeKey.SESSION_URI.getKey(), sessionUri);
243-
attributeMap.put(AttributeKey.SESSION_URI.getKey(), EventAttribute.setValue(sessionUri));
244-
245-
return sessionResponse;
239+
if (selected.isPresent()) {
240+
CreateSessionResponse sessionResponse = selected.get().get();
241+
242+
sessions.add(sessionResponse.getSession());
243+
SessionId sessionId = sessionResponse.getSession().getId();
244+
Capabilities caps = sessionResponse.getSession().getCapabilities();
245+
String sessionUri = sessionResponse.getSession().getUri().toString();
246+
SESSION_ID.accept(span, sessionId);
247+
CAPABILITIES.accept(span, caps);
248+
SESSION_ID_EVENT.accept(attributeMap, sessionId);
249+
CAPABILITIES_EVENT.accept(attributeMap, caps);
250+
span.setAttribute(AttributeKey.SESSION_URI.getKey(), sessionUri);
251+
attributeMap.put(AttributeKey.SESSION_URI.getKey(), EventAttribute.setValue(sessionUri));
252+
253+
span.addEvent("Session created by the distributor", attributeMap);
254+
return Either.right(sessionResponse);
255+
256+
} else {
257+
String errorMessage =
258+
String.format(
259+
"Unable to find provider for session: %s",
260+
payload.stream().map(Capabilities::toString).collect(Collectors.joining(", ")));
261+
SessionNotCreatedException exception = new RetrySessionRequestException(errorMessage);
262+
return Either.left(exception);
263+
}
246264
} catch (SessionNotCreatedException e) {
247265
span.setAttribute("error", true);
248266
span.setStatus(Status.ABORTED);
@@ -252,7 +270,7 @@ public CreateSessionResponse newSession(HttpRequest request)
252270
EventAttribute.setValue("Unable to create session: " + e.getMessage()));
253271
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
254272

255-
throw e;
273+
return Either.left(e);
256274
} catch (IOException e) {
257275
span.setAttribute("error", true);
258276
span.setStatus(Status.UNKNOWN);
@@ -262,7 +280,7 @@ public CreateSessionResponse newSession(HttpRequest request)
262280
EventAttribute.setValue("Unknown error in LocalDistributor while creating session: " + e.getMessage()));
263281
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
264282

265-
throw new SessionNotCreatedException(e.getMessage(), e);
283+
return Either.left(new SessionNotCreatedException(e.getMessage(), e));
266284
} finally {
267285
span.close();
268286
}

‎java/server/src/org/openqa/selenium/grid/distributor/RetrySessionRequestException.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.grid.distributor;
19+
20+
import org.openqa.selenium.SessionNotCreatedException;
21+
22+
public class RetrySessionRequestException extends SessionNotCreatedException {
23+
public RetrySessionRequestException(String msg) {
24+
super(msg);
25+
}
26+
27+
public RetrySessionRequestException(String msg, Throwable cause) {
28+
super(msg, cause);
29+
}
30+
}

‎java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import static org.openqa.selenium.grid.config.StandardGridRoles.EVENT_BUS_ROLE;
4545
import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
4646
import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_MAP_ROLE;
47+
import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUER_ROLE;
4748
import static org.openqa.selenium.remote.http.HttpMethod.GET;
4849
import static org.openqa.selenium.remote.http.Route.get;
4950

@@ -65,7 +66,7 @@ public String getDescription() {
6566

6667
@Override
6768
public Set<Role> getConfigurableRoles() {
68-
return ImmutableSet.of(EVENT_BUS_ROLE, HTTPD_ROLE, SESSION_MAP_ROLE);
69+
return ImmutableSet.of(EVENT_BUS_ROLE, HTTPD_ROLE, SESSION_MAP_ROLE, SESSION_QUEUER_ROLE);
6970
}
7071

7172
@Override

‎java/server/src/org/openqa/selenium/grid/distributor/local/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
artifact("com.google.guava:guava"),
2932
],
3033
)

0 commit comments

Comments
 (0)