Skip to content

Commit 7b832ea

Browse files
pujaganipujagani
authored
[grid] Ensure Distributor rejects request immediately if no matching slot exists in the Grid
1 parent 5a943d5 commit 7b832ea

File tree

9 files changed

+193
-27
lines changed

9 files changed

+193
-27
lines changed

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
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.data;
19+
20+
import org.openqa.selenium.Capabilities;
21+
import org.openqa.selenium.internal.Require;
22+
import org.openqa.selenium.json.JsonInput;
23+
import org.openqa.selenium.json.TypeToken;
24+
25+
import java.lang.reflect.Type;
26+
import java.util.HashMap;
27+
import java.util.LinkedHashSet;
28+
import java.util.Map;
29+
import java.util.Objects;
30+
import java.util.Set;
31+
import java.util.StringJoiner;
32+
33+
import static java.util.Collections.unmodifiableMap;
34+
import static java.util.Collections.unmodifiableSet;
35+
36+
public class SessionRequestCapability {
37+
38+
private static final Type SET_OF_CAPABILITIES = new TypeToken<Set<Capabilities>>() {
39+
}.getType();
40+
private final RequestId requestId;
41+
private final Set<Capabilities> desiredCapabilities;
42+
43+
public SessionRequestCapability(
44+
RequestId requestId,
45+
Set<Capabilities> desiredCapabilities) {
46+
this.requestId = Require.nonNull("Request ID", requestId);
47+
this.desiredCapabilities = unmodifiableSet(
48+
new LinkedHashSet<>(Require.nonNull("Capabilities", desiredCapabilities)));
49+
}
50+
51+
public RequestId getRequestId() {
52+
return requestId;
53+
}
54+
55+
public Set<Capabilities> getDesiredCapabilities() {
56+
return desiredCapabilities;
57+
}
58+
59+
@Override
60+
public String toString() {
61+
return new StringJoiner(", ", SessionRequestCapability.class.getSimpleName() + "[", "]")
62+
.add("requestId=" + requestId)
63+
.add("desiredCapabilities=" + desiredCapabilities)
64+
.toString();
65+
}
66+
67+
@Override
68+
public boolean equals(Object o) {
69+
if (!(o instanceof SessionRequestCapability)) {
70+
return false;
71+
}
72+
SessionRequestCapability that = (SessionRequestCapability) o;
73+
74+
return this.requestId.equals(that.requestId) &&
75+
this.desiredCapabilities.equals(that.desiredCapabilities);
76+
}
77+
78+
@Override
79+
public int hashCode() {
80+
return Objects.hash(requestId, desiredCapabilities);
81+
}
82+
83+
private Map<String, Object> toJson() {
84+
Map<String, Object> toReturn = new HashMap<>();
85+
toReturn.put("requestId", requestId);
86+
toReturn.put("capabilities", desiredCapabilities);
87+
return unmodifiableMap(toReturn);
88+
}
89+
90+
private static SessionRequestCapability fromJson(JsonInput input) {
91+
RequestId id = null;
92+
Set<Capabilities> capabilities = null;
93+
94+
input.beginObject();
95+
while (input.hasNext()) {
96+
switch (input.nextName()) {
97+
case "capabilities":
98+
capabilities = input.read(SET_OF_CAPABILITIES);
99+
break;
100+
101+
case "requestId":
102+
id = input.read(RequestId.class);
103+
break;
104+
105+
default:
106+
input.skipValue();
107+
break;
108+
}
109+
}
110+
input.endObject();
111+
112+
return new SessionRequestCapability(id, capabilities);
113+
}
114+
}

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

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.openqa.selenium.grid.data.NodeStatusEvent;
4040
import org.openqa.selenium.grid.data.RequestId;
4141
import org.openqa.selenium.grid.data.SessionRequest;
42+
import org.openqa.selenium.grid.data.SessionRequestCapability;
4243
import org.openqa.selenium.grid.data.Slot;
4344
import org.openqa.selenium.grid.data.SlotId;
4445
import org.openqa.selenium.grid.data.TraceSessionRequest;
@@ -369,21 +370,6 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Sess
369370
return Either.left(exception);
370371
}
371372

372-
// If there are capabilities, but the Grid doesn't support them, bail too.
373-
long unmatchableCount = request.getDesiredCapabilities().stream()
374-
.filter(caps -> !isSupported(caps))
375-
.count();
376-
if (unmatchableCount == request.getDesiredCapabilities().size()) {
377-
SessionNotCreatedException exception = new SessionNotCreatedException(
378-
"No nodes support the capabilities in the request");
379-
EXCEPTION.accept(attributeMap, exception);
380-
attributeMap.put(
381-
AttributeKey.EXCEPTION_MESSAGE.getKey(),
382-
EventAttribute.setValue(exception.getMessage()));
383-
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
384-
return Either.left(exception);
385-
}
386-
387373
boolean retry = false;
388374
SessionNotCreatedException lastFailure = new SessionNotCreatedException("Unable to create new session");
389375
for (Capabilities caps : request.getDesiredCapabilities()) {
@@ -555,6 +541,9 @@ public class NewSessionRunnable implements Runnable {
555541

556542
@Override
557543
public void run() {
544+
List<SessionRequestCapability> queueContents = sessionQueue.getQueueContents();
545+
checkMatchingSlot(queueContents);
546+
558547
int initialSize = sessionQueue.getQueueContents().size();
559548
boolean retry = initialSize != 0;
560549

@@ -583,6 +572,20 @@ public void run() {
583572
}
584573
}
585574

575+
private void checkMatchingSlot(List<SessionRequestCapability> sessionRequests) {
576+
for(SessionRequestCapability request : sessionRequests) {
577+
long unmatchableCount = request.getDesiredCapabilities().stream()
578+
.filter(caps -> !isSupported(caps))
579+
.count();
580+
581+
if (unmatchableCount == request.getDesiredCapabilities().size()) {
582+
SessionNotCreatedException exception = new SessionNotCreatedException(
583+
"No nodes support the capabilities in the request");
584+
sessionQueue.complete(request.getRequestId(), Either.left(exception));
585+
}
586+
}
587+
}
588+
586589
private void handleNewSessionRequest(SessionRequest sessionRequest) {
587590
RequestId reqId = sessionRequest.getRequestId();
588591

‎java/server/src/org/openqa/selenium/grid/graphql/Grid.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.openqa.selenium.ImmutableCapabilities;
2424
import org.openqa.selenium.grid.data.DistributorStatus;
2525
import org.openqa.selenium.grid.data.NodeStatus;
26+
import org.openqa.selenium.grid.data.SessionRequestCapability;
2627
import org.openqa.selenium.grid.data.Slot;
2728
import org.openqa.selenium.grid.distributor.Distributor;
2829
import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
@@ -55,7 +56,11 @@ public Grid(
5556
Require.nonNull("Distributor", distributor);
5657
this.uri = Require.nonNull("Grid's public URI", uri);
5758
NewSessionQueue sessionQueue = Require.nonNull("New session queue", newSessionQueue);
58-
this.queueInfoList = sessionQueue.getQueueContents();
59+
this.queueInfoList = sessionQueue
60+
.getQueueContents()
61+
.stream()
62+
.map(SessionRequestCapability::getDesiredCapabilities)
63+
.collect(Collectors.toList());
5964
this.distributorStatus = Suppliers.memoize(distributor::getStatus);
6065
this.version = Require.nonNull("Grid's version", version);
6166
}

‎java/server/src/org/openqa/selenium/grid/sessionqueue/NewSessionQueue.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.openqa.selenium.grid.data.CreateSessionResponse;
2323
import org.openqa.selenium.grid.data.RequestId;
2424
import org.openqa.selenium.grid.data.SessionRequest;
25+
import org.openqa.selenium.grid.data.SessionRequestCapability;
2526
import org.openqa.selenium.grid.security.RequiresSecretFilter;
2627
import org.openqa.selenium.grid.security.Secret;
2728
import org.openqa.selenium.internal.Either;
@@ -72,7 +73,7 @@ protected NewSessionQueue(Tracer tracer, Secret registrationSecret) {
7273
post("/se/grid/newsessionqueue/session/{requestId}/retry")
7374
.to(params -> new AddBackToSessionQueue(tracer, this, requestIdFrom(params)))
7475
.with(requiresSecret),
75-
post("/se/grid/newsessionqueue/session/{requestId}/fail")
76+
post("/se/grid/newsessionqueue/session/{requestId}/failure")
7677
.to(params -> new SessionNotCreated(tracer, this, requestIdFrom(params)))
7778
.with(requiresSecret),
7879
post("/se/grid/newsessionqueue/session/{requestId}/success")
@@ -107,7 +108,7 @@ private RequestId requestIdFrom(Map<String, String> params) {
107108

108109
public abstract int clearQueue();
109110

110-
public abstract List<Set<Capabilities>> getQueueContents();
111+
public abstract List<SessionRequestCapability> getQueueContents();
111112

112113
@Override
113114
public boolean matches(HttpRequest req) {

‎java/server/src/org/openqa/selenium/grid/sessionqueue/SessionNotCreated.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
5151
try (Span span = newSpanAsChildOf(tracer, req, "sessionqueue.created_bad")) {
5252
HTTP_REQUEST.accept(span, req);
5353

54-
SessionNotCreatedException exception = Contents.fromJson(req, SessionNotCreatedException.class);
54+
String message = Contents.fromJson(req, String.class);
55+
SessionNotCreatedException exception = new SessionNotCreatedException(message);
5556
queue.complete(requestId, Either.left(exception));
5657

5758
HttpResponse res = new HttpResponse();

‎java/server/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.openqa.selenium.grid.data.NewSessionRequestEvent;
1212
import org.openqa.selenium.grid.data.RequestId;
1313
import org.openqa.selenium.grid.data.SessionRequest;
14+
import org.openqa.selenium.grid.data.SessionRequestCapability;
1415
import org.openqa.selenium.grid.data.TraceSessionRequest;
1516
import org.openqa.selenium.grid.data.SlotMatcher;
1617
import org.openqa.selenium.grid.distributor.config.DistributorOptions;
@@ -379,13 +380,14 @@ public int clearQueue() {
379380
}
380381

381382
@Override
382-
public List<Set<Capabilities>> getQueueContents() {
383+
public List<SessionRequestCapability> getQueueContents() {
383384
Lock readLock = lock.readLock();
384385
readLock.lock();
385386

386387
try {
387388
return queue.stream()
388-
.map(SessionRequest::getDesiredCapabilities)
389+
.map(req ->
390+
new SessionRequestCapability(req.getRequestId(), req.getDesiredCapabilities()))
389391
.collect(Collectors.toList());
390392
} finally {
391393
readLock.unlock();

‎java/server/src/org/openqa/selenium/grid/sessionqueue/remote/RemoteNewSessionQueue.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.openqa.selenium.grid.config.Config;
2323
import org.openqa.selenium.grid.data.CreateSessionResponse;
2424
import org.openqa.selenium.grid.data.RequestId;
25+
import org.openqa.selenium.grid.data.SessionRequestCapability;
2526
import org.openqa.selenium.grid.log.LoggingOptions;
2627
import org.openqa.selenium.grid.security.AddSecretFilter;
2728
import org.openqa.selenium.grid.security.Secret;
@@ -57,7 +58,7 @@
5758

5859
public class RemoteNewSessionQueue extends NewSessionQueue {
5960

60-
private static final Type QUEUE_CONTENTS_TYPE = new TypeToken<List<Set<Capabilities>>>() {}.getType();
61+
private static final Type QUEUE_CONTENTS_TYPE = new TypeToken<List<SessionRequestCapability>>() {}.getType();
6162
private static final Json JSON = new Json();
6263
private final HttpClient client;
6364
private final Filter addSecret;
@@ -151,7 +152,7 @@ public void complete(RequestId reqId, Either<SessionNotCreatedException, CreateS
151152
.setContent(Contents.asJson(result.right()));
152153
} else {
153154
upstream = new HttpRequest(POST, String.format("/se/grid/newsessionqueue/session/%s/failure", reqId))
154-
.setContent(Contents.asJson(result.left()));
155+
.setContent(Contents.asJson(result.left().getRawMessage()));
155156
}
156157

157158
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
@@ -168,7 +169,7 @@ public int clearQueue() {
168169
}
169170

170171
@Override
171-
public List<Set<Capabilities>> getQueueContents() {
172+
public List<SessionRequestCapability> getQueueContents() {
172173
HttpRequest upstream = new HttpRequest(GET, "/se/grid/newsessionqueue/queue");
173174
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
174175
HttpResponse response = client.execute(upstream);

‎java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import java.util.Set;
6969
import java.util.UUID;
7070
import java.util.concurrent.CountDownLatch;
71+
import java.util.concurrent.atomic.AtomicInteger;
7172

7273
import static java.util.Collections.singletonList;
7374
import static java.util.Collections.singletonMap;
@@ -177,7 +178,23 @@ private void continueOnceAddedToQueue(SessionRequest request) {
177178
}
178179

179180
@Test
180-
public void shouldBeAbleToGetSessionQueueSize() {
181+
public void shouldBeAbleToGetSessionQueueSize() throws URISyntaxException {
182+
AtomicInteger count = new AtomicInteger();
183+
184+
URI nodeUri = new URI("http://example:5678");
185+
TestSessionFactory sessionFactory = new TestSessionFactory((id, caps) -> new Session(
186+
id,
187+
nodeUri,
188+
new ImmutableCapabilities(),
189+
caps,
190+
Instant.now()));
191+
192+
LocalNode localNode = LocalNode.builder(tracer, events, nodeUri, nodeUri, registrationSecret)
193+
.add(caps, sessionFactory).build();
194+
distributor.add(localNode);
195+
196+
wait.until(obj -> distributor.getStatus().hasCapacity());
197+
181198
SessionRequest request = new SessionRequest(
182199
new RequestId(UUID.randomUUID()),
183200
Instant.now(),
@@ -186,6 +203,7 @@ public void shouldBeAbleToGetSessionQueueSize() {
186203
Map.of(),
187204
Map.of());
188205

206+
continueOnceAddedToQueue(request);
189207
continueOnceAddedToQueue(request);
190208
GraphqlHandler handler = new GraphqlHandler(tracer, distributor, queue, publicUri, version);
191209

@@ -199,7 +217,22 @@ public void shouldBeAbleToGetSessionQueueSize() {
199217
}
200218

201219
@Test
202-
public void shouldBeAbleToGetSessionQueueRequests() {
220+
public void shouldBeAbleToGetSessionQueueRequests() throws URISyntaxException {
221+
AtomicInteger count = new AtomicInteger();
222+
223+
URI nodeUri = new URI("http://example:5678");
224+
TestSessionFactory sessionFactory = new TestSessionFactory((id, caps) -> new Session(
225+
id,
226+
nodeUri,
227+
new ImmutableCapabilities(),
228+
caps,
229+
Instant.now()));
230+
231+
LocalNode localNode = LocalNode.builder(tracer, events, nodeUri, nodeUri, registrationSecret)
232+
.add(caps, sessionFactory).build();
233+
distributor.add(localNode);
234+
wait.until(obj -> distributor.getStatus().hasCapacity());
235+
203236
SessionRequest request = new SessionRequest(
204237
new RequestId(UUID.randomUUID()),
205238
Instant.now(),
@@ -209,6 +242,7 @@ public void shouldBeAbleToGetSessionQueueRequests() {
209242
Map.of());
210243

211244
continueOnceAddedToQueue(request);
245+
continueOnceAddedToQueue(request);
212246

213247
GraphqlHandler handler = new GraphqlHandler(tracer, distributor, queue, publicUri, version);
214248

‎java/server/test/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueueTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.openqa.selenium.grid.data.NewSessionRequestEvent;
3636
import org.openqa.selenium.grid.data.RequestId;
3737
import org.openqa.selenium.grid.data.Session;
38+
import org.openqa.selenium.grid.data.SessionRequestCapability;
3839
import org.openqa.selenium.grid.security.Secret;
3940
import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
4041
import org.openqa.selenium.grid.data.SessionRequest;
@@ -72,6 +73,7 @@
7273
import java.util.concurrent.atomic.AtomicInteger;
7374
import java.util.concurrent.atomic.AtomicReference;
7475
import java.util.function.Supplier;
76+
import java.util.stream.Collectors;
7577

7678
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
7779
import static java.net.HttpURLConnection.HTTP_OK;
@@ -240,7 +242,10 @@ public void shouldBeClearQueue() {
240242
public void shouldBeAbleToGetQueueContents() {
241243
localQueue.injectIntoQueue(sessionRequest);
242244

243-
List<Set<Capabilities>> response = queue.getQueueContents();
245+
List<Set<Capabilities>> response = queue.getQueueContents()
246+
.stream()
247+
.map(SessionRequestCapability::getDesiredCapabilities)
248+
.collect(Collectors.toList());
244249

245250
assertThat(response).hasSize(1);
246251

0 commit comments

Comments
 (0)