Skip to content

Commit 554873d

Browse files
shs96cshs96c
committed
Rework the LocalNewSessionQueue to be simpler
1 parent 83e80c2 commit 554873d

File tree

14 files changed

+721
-1082
lines changed

14 files changed

+721
-1082
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ public class NewSessionResponse {
3232
private final Session session;
3333
private final byte[] downstreamEncodedResponse;
3434

35-
public NewSessionResponse(RequestId requestId, Session session,
36-
byte[] downstreamEncodedResponse) {
35+
public NewSessionResponse(
36+
RequestId requestId,
37+
Session session,
38+
byte[] downstreamEncodedResponse) {
3739
this.requestId = Require.nonNull("Request Id", requestId);
3840
this.session = Require.nonNull("Session", session);
3941
this.downstreamEncodedResponse = Require.nonNull

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

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@
3333
import org.openqa.selenium.grid.data.NewSessionErrorResponse;
3434
import org.openqa.selenium.grid.data.NewSessionRejectedEvent;
3535
import org.openqa.selenium.grid.data.NewSessionRequestEvent;
36-
import org.openqa.selenium.grid.data.NewSessionResponse;
37-
import org.openqa.selenium.grid.data.NewSessionResponseEvent;
3836
import org.openqa.selenium.grid.data.NodeAddedEvent;
3937
import org.openqa.selenium.grid.data.NodeDrainComplete;
4038
import org.openqa.selenium.grid.data.NodeHeartBeatEvent;
@@ -113,14 +111,14 @@ public class LocalDistributor extends Distributor {
113111
private final GridModel model;
114112
private final Map<NodeId, Node> nodes;
115113

116-
private final NewSessionQueue sessionRequests;
114+
private final NewSessionQueue sessionQueue;
117115

118116
public LocalDistributor(
119117
Tracer tracer,
120118
EventBus bus,
121119
HttpClient.Factory clientFactory,
122120
SessionMap sessions,
123-
NewSessionQueue sessionRequests,
121+
NewSessionQueue sessionQueue,
124122
Secret registrationSecret,
125123
Duration healthcheckInterval) {
126124
super(tracer, clientFactory, new DefaultSlotSelector(), sessions, registrationSecret);
@@ -130,7 +128,7 @@ public LocalDistributor(
130128
this.sessions = Require.nonNull("Session map", sessions);
131129
this.model = new GridModel(bus);
132130
this.nodes = new ConcurrentHashMap<>();
133-
this.sessionRequests = Require.nonNull("New Session Request Queue", sessionRequests);
131+
this.sessionQueue = Require.nonNull("New Session Request Queue", sessionQueue);
134132
this.registrationSecret = Require.nonNull("Registration secret", registrationSecret);
135133
this.healthcheckInterval = Require.nonNull("Health check interval", healthcheckInterval);
136134

@@ -169,15 +167,14 @@ public static Distributor create(Config config) {
169167
HttpClient.Factory clientFactory = new NetworkOptions(config).getHttpClientFactory(tracer);
170168
SessionMap sessions = new SessionMapOptions(config).getSessionMap();
171169
SecretOptions secretOptions = new SecretOptions(config);
172-
NewSessionQueue sessionRequests =
173-
new NewSessionQueueOptions(config).getSessionQueue(
174-
"org.openqa.selenium.grid.sessionqueue.remote.RemoteNewSessionQueue");
170+
NewSessionQueue sessionQueue = new NewSessionQueueOptions(config).getSessionQueue(
171+
"org.openqa.selenium.grid.sessionqueue.remote.RemoteNewSessionQueue");
175172
return new LocalDistributor(
176173
tracer,
177174
bus,
178175
clientFactory,
179176
sessions,
180-
sessionRequests,
177+
sessionQueue,
181178
secretOptions.getRegistrationSecret(),
182179
distributorOptions.getHealthCheckInterval());
183180
}
@@ -393,7 +390,7 @@ public void run() {
393390
if (hasCapacity) {
394391
RequestId reqId = requestIds.poll();
395392
if (reqId != null) {
396-
Optional<SessionRequest> maybeRequest = sessionRequests.remove(reqId);
393+
Optional<SessionRequest> maybeRequest = sessionQueue.remove(reqId);
397394
// Check if polling the queue did not return null
398395
if (maybeRequest.isPresent()) {
399396
handleNewSessionRequest(maybeRequest.get(), reqId);
@@ -422,34 +419,20 @@ private void handleNewSessionRequest(SessionRequest sessionRequest, RequestId re
422419
EventAttribute.setValue(reqId.toString()));
423420

424421
attributeMap.put("request", EventAttribute.setValue(sessionRequest.toString()));
425-
Either<SessionNotCreatedException, CreateSessionResponse> response =
426-
newSession(sessionRequest);
427-
if (response.isRight()) {
428-
CreateSessionResponse sessionResponse = response.right();
429-
NewSessionResponse newSessionResponse =
430-
new NewSessionResponse(
431-
reqId,
432-
sessionResponse.getSession(),
433-
sessionResponse.getDownstreamEncodedResponse());
434-
435-
bus.fire(new NewSessionResponseEvent(newSessionResponse));
436-
} else {
437-
SessionNotCreatedException exception = response.left();
422+
Either<SessionNotCreatedException, CreateSessionResponse> response = newSession(sessionRequest);
438423

439-
if (exception instanceof RetrySessionRequestException) {
440-
boolean retried = sessionRequests.retryAddToQueue(sessionRequest);
424+
if (response.isLeft()) {
425+
boolean retried = sessionQueue.retryAddToQueue(sessionRequest);
441426

442-
attributeMap.put("request.retry_add", EventAttribute.setValue(retried));
443-
span.addEvent("Retry adding to front of queue. No slot available.", attributeMap);
427+
attributeMap.put("request.retry_add", EventAttribute.setValue(retried));
428+
span.addEvent("Retry adding to front of queue. No slot available.", attributeMap);
444429

445-
if (!retried) {
446-
span.addEvent("Retry adding to front of queue failed.", attributeMap);
447-
fireSessionRejectedEvent(exception.getMessage(), reqId);
448-
}
449-
} else {
450-
fireSessionRejectedEvent(exception.getMessage(), reqId);
430+
if (retried) {
431+
return;
451432
}
452433
}
434+
435+
sessionQueue.complete(reqId, response);
453436
}
454437
}
455438

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@
1818
package org.openqa.selenium.grid.sessionqueue;
1919

2020
import org.openqa.selenium.Capabilities;
21+
import org.openqa.selenium.SessionNotCreatedException;
22+
import org.openqa.selenium.grid.data.CreateSessionResponse;
2123
import org.openqa.selenium.grid.data.RequestId;
2224
import org.openqa.selenium.grid.security.RequiresSecretFilter;
2325
import org.openqa.selenium.grid.security.Secret;
26+
import org.openqa.selenium.internal.Either;
2427
import org.openqa.selenium.internal.Require;
2528
import org.openqa.selenium.remote.NewSessionPayload;
2629
import org.openqa.selenium.remote.http.Contents;
@@ -77,9 +80,13 @@ protected NewSessionQueue(Tracer tracer, Secret registrationSecret) {
7780
.to(() -> new OfferLastToSessionQueue(tracer, this)),
7881
post("/se/grid/newsessionqueue/session")
7982
.to(() -> new AddToSessionQueue(tracer, this)),
80-
post("/se/grid/newsessionqueue/session/retry/{requestId}")
83+
post("/se/grid/newsessionqueue/session/{requestId}/retry")
8184
.to(params -> new AddBackToSessionQueue(tracer, this, requestIdFrom(params)))
8285
.with(requiresSecret),
86+
post("/se/grid/newsessionqueue/session/{requestId}/fail")
87+
.to(params -> new SessionNotCreated(tracer, this, requestIdFrom(params))),
88+
post("/se/grid/newsessionqueue/session/{requestId}/success")
89+
.to(params -> new SessionCreated(tracer, this, requestIdFrom(params))),
8390
get("/se/grid/newsessionqueue/session/{requestId}")
8491
.to(params -> new RemoveFromSessionQueue(tracer, this, requestIdFrom(params)))
8592
.with(requiresSecret),
@@ -102,6 +109,8 @@ private RequestId requestIdFrom(Map<String, String> params) {
102109

103110
public abstract Optional<SessionRequest> remove(RequestId reqId);
104111

112+
public abstract void complete(RequestId reqId, Either<SessionNotCreatedException, CreateSessionResponse> result);
113+
105114
public abstract int clearQueue();
106115

107116
public abstract List<Set<Capabilities>> getQueueContents();

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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.sessionqueue;
19+
20+
import org.openqa.selenium.grid.data.CreateSessionResponse;
21+
import org.openqa.selenium.grid.data.RequestId;
22+
import org.openqa.selenium.internal.Either;
23+
import org.openqa.selenium.internal.Require;
24+
import org.openqa.selenium.remote.http.Contents;
25+
import org.openqa.selenium.remote.http.HttpHandler;
26+
import org.openqa.selenium.remote.http.HttpRequest;
27+
import org.openqa.selenium.remote.http.HttpResponse;
28+
import org.openqa.selenium.remote.tracing.Span;
29+
import org.openqa.selenium.remote.tracing.Tracer;
30+
31+
import java.io.UncheckedIOException;
32+
33+
import static org.openqa.selenium.remote.tracing.HttpTracing.newSpanAsChildOf;
34+
import static org.openqa.selenium.remote.tracing.Tags.HTTP_REQUEST;
35+
import static org.openqa.selenium.remote.tracing.Tags.HTTP_RESPONSE;
36+
37+
class SessionCreated implements HttpHandler {
38+
private final Tracer tracer;
39+
private final NewSessionQueue queue;
40+
private final RequestId requestId;
41+
42+
public SessionCreated(Tracer tracer, NewSessionQueue queue, RequestId requestId) {
43+
this.tracer = Require.nonNull("Tracer", tracer);
44+
this.queue = Require.nonNull("New Session Queue", queue);
45+
this.requestId = Require.nonNull("Request ID", requestId);
46+
}
47+
48+
@Override
49+
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
50+
try (Span span = newSpanAsChildOf(tracer, req, "sessionqueue.created_ok")) {
51+
HTTP_REQUEST.accept(span, req);
52+
53+
CreateSessionResponse response = Contents.fromJson(req, CreateSessionResponse.class);
54+
queue.complete(requestId, Either.right(response));
55+
56+
HttpResponse res = new HttpResponse();
57+
HTTP_RESPONSE.accept(span, res);
58+
return res;
59+
}
60+
}
61+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
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.sessionqueue;
19+
20+
import org.openqa.selenium.SessionNotCreatedException;
21+
import org.openqa.selenium.grid.data.RequestId;
22+
import org.openqa.selenium.internal.Either;
23+
import org.openqa.selenium.internal.Require;
24+
import org.openqa.selenium.remote.http.Contents;
25+
import org.openqa.selenium.remote.http.HttpHandler;
26+
import org.openqa.selenium.remote.http.HttpRequest;
27+
import org.openqa.selenium.remote.http.HttpResponse;
28+
import org.openqa.selenium.remote.tracing.Span;
29+
import org.openqa.selenium.remote.tracing.Tracer;
30+
31+
import java.io.UncheckedIOException;
32+
33+
import static org.openqa.selenium.remote.tracing.HttpTracing.newSpanAsChildOf;
34+
import static org.openqa.selenium.remote.tracing.Tags.HTTP_REQUEST;
35+
import static org.openqa.selenium.remote.tracing.Tags.HTTP_RESPONSE;
36+
37+
class SessionNotCreated implements HttpHandler {
38+
39+
private final Tracer tracer;
40+
private final NewSessionQueue queue;
41+
private final RequestId requestId;
42+
43+
public SessionNotCreated(Tracer tracer, NewSessionQueue queue, RequestId requestId) {
44+
this.tracer = Require.nonNull("Tracer", tracer);
45+
this.queue = Require.nonNull("New Session Queue", queue);
46+
this.requestId = Require.nonNull("Request ID", requestId);
47+
}
48+
49+
@Override
50+
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
51+
try (Span span = newSpanAsChildOf(tracer, req, "sessionqueue.created_bad")) {
52+
HTTP_REQUEST.accept(span, req);
53+
54+
SessionNotCreatedException exception = Contents.fromJson(req, SessionNotCreatedException.class);
55+
queue.complete(requestId, Either.left(exception));
56+
57+
HttpResponse res = new HttpResponse();
58+
HTTP_RESPONSE.accept(span, res);
59+
return res;
60+
}
61+
}
62+
}

0 commit comments

Comments
 (0)