Skip to content

Commit e9e684d

Browse files
joerg1985VietND96
andauthored
[grid] limit the number of websocket connections per session (#14410)
Co-authored-by: Viet Nguyen Duc <[email protected]>
1 parent b01041f commit e9e684d

File tree

11 files changed

+154
-5
lines changed

11 files changed

+154
-5
lines changed

java/src/org/openqa/selenium/grid/node/Node.java

+11
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@
101101
* by {@code sessionId}. This returns a boolean.</td>
102102
* </tr>
103103
* <tr>
104+
* <td>POST</td>
105+
* <td>/se/grid/node/connection/{sessionId}</td>
106+
* <td>Allows the node to be ask about whether or not new websocket connections are allowed for the {@link Session}
107+
* identified by {@code sessionId}. This returns a boolean.</td>
108+
* </tr>
109+
* <tr>
104110
* <td>*</td>
105111
* <td>/session/{sessionId}/*</td>
106112
* <td>The request is forwarded to the {@link Session} identified by {@code sessionId}. When the
@@ -172,6 +178,9 @@ protected Node(
172178
get("/se/grid/node/owner/{sessionId}")
173179
.to(params -> new IsSessionOwner(this, sessionIdFrom(params)))
174180
.with(spanDecorator("node.is_session_owner").andThen(requiresSecret)),
181+
post("/se/grid/node/connection/{sessionId}")
182+
.to(params -> new TryAcquireConnection(this, sessionIdFrom(params)))
183+
.with(spanDecorator("node.is_session_owner").andThen(requiresSecret)),
175184
delete("/se/grid/node/session/{sessionId}")
176185
.to(params -> new StopNodeSession(this, sessionIdFrom(params)))
177186
.with(spanDecorator("node.stop_session").andThen(requiresSecret)),
@@ -244,6 +253,8 @@ public TemporaryFilesystem getDownloadsFilesystem(UUID uuid) throws IOException
244253

245254
public abstract boolean isSessionOwner(SessionId id);
246255

256+
public abstract boolean tryAcquireConnection(SessionId id);
257+
247258
public abstract boolean isSupporting(Capabilities capabilities);
248259

249260
public abstract NodeStatus getStatus();

java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java

+7
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ public Optional<Consumer<Message>> apply(String uri, Consumer<Message> downstrea
9494
return Optional.empty();
9595
}
9696

97+
// ensure one session does not open to many connections, this might have a negative impact on
98+
// the grid health
99+
if (!node.tryAcquireConnection(id)) {
100+
LOG.warning("Too many websocket connections initiated by " + id);
101+
return Optional.empty();
102+
}
103+
97104
Session session = node.getSession(id);
98105
Capabilities caps = session.getCapabilities();
99106
LOG.fine("Scanning for endpoint: " + caps);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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.node;
19+
20+
import static org.openqa.selenium.remote.http.Contents.asJson;
21+
22+
import com.google.common.collect.ImmutableMap;
23+
import java.io.UncheckedIOException;
24+
import org.openqa.selenium.internal.Require;
25+
import org.openqa.selenium.remote.SessionId;
26+
import org.openqa.selenium.remote.http.HttpHandler;
27+
import org.openqa.selenium.remote.http.HttpRequest;
28+
import org.openqa.selenium.remote.http.HttpResponse;
29+
30+
class TryAcquireConnection implements HttpHandler {
31+
32+
private final Node node;
33+
private final SessionId id;
34+
35+
TryAcquireConnection(Node node, SessionId id) {
36+
this.node = Require.nonNull("Node", node);
37+
this.id = Require.nonNull("Session id", id);
38+
}
39+
40+
@Override
41+
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
42+
return new HttpResponse()
43+
.setContent(asJson(ImmutableMap.of("value", node.tryAcquireConnection(id))));
44+
}
45+
}

java/src/org/openqa/selenium/grid/node/config/NodeFlags.java

+9
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.openqa.selenium.grid.node.config;
1919

2020
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
21+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_CONNECTION_LIMIT;
2122
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
2223
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DRAIN_AFTER_SESSION_COUNT;
2324
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_ENABLE_BIDI;
@@ -77,6 +78,14 @@ public class NodeFlags implements HasRoles {
7778
@ConfigValue(section = NODE_SECTION, name = "session-timeout", example = "60")
7879
public int sessionTimeout = DEFAULT_SESSION_TIMEOUT;
7980

81+
@Parameter(
82+
names = {"--connection-limit-per-session"},
83+
description =
84+
"Let X be the maximum number of websocket connections per session.This will ensure one"
85+
+ " session is not able to exhaust the connection limit of the host")
86+
@ConfigValue(section = NODE_SECTION, name = "connection-limit-per-session", example = "8")
87+
public int connectionLimitPerSession = DEFAULT_CONNECTION_LIMIT;
88+
8089
@Parameter(
8190
names = {"--detect-drivers"},
8291
arity = 1,

java/src/org/openqa/selenium/grid/node/config/NodeOptions.java

+10
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public class NodeOptions {
7373
public static final int DEFAULT_HEARTBEAT_PERIOD = 60;
7474
public static final int DEFAULT_SESSION_TIMEOUT = 300;
7575
public static final int DEFAULT_DRAIN_AFTER_SESSION_COUNT = 0;
76+
public static final int DEFAULT_CONNECTION_LIMIT = 10;
7677
public static final boolean DEFAULT_ENABLE_CDP = true;
7778
public static final boolean DEFAULT_ENABLE_BIDI = true;
7879
static final String NODE_SECTION = "node";
@@ -262,6 +263,15 @@ public int getMaxSessions() {
262263
return Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
263264
}
264265

266+
public int getConnectionLimitPerSession() {
267+
int connectionLimit =
268+
config
269+
.getInt(NODE_SECTION, "connection-limit-per-session")
270+
.orElse(DEFAULT_CONNECTION_LIMIT);
271+
Require.positive("Session connection limit", connectionLimit);
272+
return connectionLimit;
273+
}
274+
265275
public Duration getSessionTimeout() {
266276
// If the user sets 10s or less, we default to 10s.
267277
int seconds =

java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java

+13-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.Optional;
3535
import java.util.ServiceLoader;
3636
import java.util.UUID;
37+
import java.util.concurrent.atomic.AtomicInteger;
3738
import java.util.logging.Logger;
3839
import java.util.stream.StreamSupport;
3940
import org.openqa.selenium.Capabilities;
@@ -98,6 +99,8 @@ public class OneShotNode extends Node {
9899
private final Duration heartbeatPeriod;
99100
private final URI gridUri;
100101
private final UUID slotId = UUID.randomUUID();
102+
private final int connectionLimitPerSession;
103+
private final AtomicInteger connectionCounter = new AtomicInteger();
101104
private RemoteWebDriver driver;
102105
private SessionId sessionId;
103106
private HttpClient client;
@@ -114,14 +117,16 @@ private OneShotNode(
114117
URI uri,
115118
URI gridUri,
116119
Capabilities stereotype,
117-
WebDriverInfo driverInfo) {
120+
WebDriverInfo driverInfo,
121+
int connectionLimitPerSession) {
118122
super(tracer, id, uri, registrationSecret, Require.positive(sessionTimeout));
119123

120124
this.heartbeatPeriod = heartbeatPeriod;
121125
this.events = Require.nonNull("Event bus", events);
122126
this.gridUri = Require.nonNull("Public Grid URI", gridUri);
123127
this.stereotype = ImmutableCapabilities.copyOf(Require.nonNull("Stereotype", stereotype));
124128
this.driverInfo = Require.nonNull("Driver info", driverInfo);
129+
this.connectionLimitPerSession = connectionLimitPerSession;
125130

126131
new JMXHelper().register(this);
127132
}
@@ -177,7 +182,8 @@ public static Node create(Config config) {
177182
.getPublicGridUri()
178183
.orElseThrow(() -> new ConfigException("Unable to determine public grid address")),
179184
stereotype,
180-
driverInfo);
185+
driverInfo,
186+
nodeOptions.getConnectionLimitPerSession());
181187
}
182188

183189
@Override
@@ -357,6 +363,11 @@ public boolean isSessionOwner(SessionId id) {
357363
return driver != null && sessionId.equals(id);
358364
}
359365

366+
@Override
367+
public boolean tryAcquireConnection(SessionId id) {
368+
return sessionId.equals(id) && connectionLimitPerSession > connectionCounter.getAndIncrement();
369+
}
370+
360371
@Override
361372
public boolean isSupporting(Capabilities capabilities) {
362373
return driverInfo.isSupporting(capabilities);

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

+31-2
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import java.util.concurrent.TimeUnit;
6060
import java.util.concurrent.atomic.AtomicBoolean;
6161
import java.util.concurrent.atomic.AtomicInteger;
62+
import java.util.concurrent.atomic.AtomicLong;
6263
import java.util.logging.Level;
6364
import java.util.logging.Logger;
6465
import java.util.stream.Collectors;
@@ -127,6 +128,7 @@ public class LocalNode extends Node {
127128
private final int configuredSessionCount;
128129
private final boolean cdpEnabled;
129130
private final boolean managedDownloadsEnabled;
131+
private final int connectionLimitPerSession;
130132

131133
private final boolean bidiEnabled;
132134
private final AtomicBoolean drainAfterSessions = new AtomicBoolean();
@@ -153,7 +155,8 @@ protected LocalNode(
153155
Duration heartbeatPeriod,
154156
List<SessionSlot> factories,
155157
Secret registrationSecret,
156-
boolean managedDownloadsEnabled) {
158+
boolean managedDownloadsEnabled,
159+
int connectionLimitPerSession) {
157160
super(
158161
tracer,
159162
new NodeId(UUID.randomUUID()),
@@ -176,6 +179,7 @@ protected LocalNode(
176179
this.cdpEnabled = cdpEnabled;
177180
this.bidiEnabled = bidiEnabled;
178181
this.managedDownloadsEnabled = managedDownloadsEnabled;
182+
this.connectionLimitPerSession = connectionLimitPerSession;
179183

180184
this.healthCheck =
181185
healthCheck == null
@@ -579,6 +583,24 @@ public boolean isSessionOwner(SessionId id) {
579583
return currentSessions.getIfPresent(id) != null;
580584
}
581585

586+
@Override
587+
public boolean tryAcquireConnection(SessionId id) throws NoSuchSessionException {
588+
SessionSlot slot = currentSessions.getIfPresent(id);
589+
590+
if (slot == null) {
591+
return false;
592+
}
593+
594+
if (connectionLimitPerSession == -1) {
595+
// no limit
596+
return true;
597+
}
598+
599+
AtomicLong counter = slot.getConnectionCounter();
600+
601+
return connectionLimitPerSession > counter.getAndIncrement();
602+
}
603+
582604
@Override
583605
public Session getSession(SessionId id) throws NoSuchSessionException {
584606
Require.nonNull("Session ID", id);
@@ -987,6 +1009,7 @@ public static class Builder {
9871009
private HealthCheck healthCheck;
9881010
private Duration heartbeatPeriod = Duration.ofSeconds(NodeOptions.DEFAULT_HEARTBEAT_PERIOD);
9891011
private boolean managedDownloadsEnabled = false;
1012+
private int connectionLimitPerSession = -1;
9901013

9911014
private Builder(Tracer tracer, EventBus bus, URI uri, URI gridUri, Secret registrationSecret) {
9921015
this.tracer = Require.nonNull("Tracer", tracer);
@@ -1041,6 +1064,11 @@ public Builder enableManagedDownloads(boolean enable) {
10411064
return this;
10421065
}
10431066

1067+
public Builder connectionLimitPerSession(int connectionLimitPerSession) {
1068+
this.connectionLimitPerSession = connectionLimitPerSession;
1069+
return this;
1070+
}
1071+
10441072
public LocalNode build() {
10451073
return new LocalNode(
10461074
tracer,
@@ -1057,7 +1085,8 @@ public LocalNode build() {
10571085
heartbeatPeriod,
10581086
factories.build(),
10591087
registrationSecret,
1060-
managedDownloadsEnabled);
1088+
managedDownloadsEnabled,
1089+
connectionLimitPerSession);
10611090
}
10621091

10631092
public Advanced advanced() {

java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public static Node create(Config config) {
7070
.enableCdp(nodeOptions.isCdpEnabled())
7171
.enableBiDi(nodeOptions.isBiDiEnabled())
7272
.enableManagedDownloads(nodeOptions.isManagedDownloadsEnabled())
73-
.heartbeatPeriod(nodeOptions.getHeartbeatPeriod());
73+
.heartbeatPeriod(nodeOptions.getHeartbeatPeriod())
74+
.connectionLimitPerSession(nodeOptions.getConnectionLimitPerSession());
7475

7576
List<DriverService.Builder<?, ?>> builders = new ArrayList<>();
7677
ServiceLoader.load(DriverService.Builder.class).forEach(builders::add);

java/src/org/openqa/selenium/grid/node/local/SessionSlot.java

+9
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.ServiceLoader;
2222
import java.util.UUID;
2323
import java.util.concurrent.atomic.AtomicBoolean;
24+
import java.util.concurrent.atomic.AtomicLong;
2425
import java.util.function.Function;
2526
import java.util.function.Predicate;
2627
import java.util.logging.Level;
@@ -59,6 +60,7 @@ public class SessionSlot
5960
private final AtomicBoolean reserved = new AtomicBoolean(false);
6061
private final boolean supportingCdp;
6162
private final boolean supportingBiDi;
63+
private final AtomicLong connectionCounter;
6264
private ActiveSession currentSession;
6365

6466
public SessionSlot(EventBus bus, Capabilities stereotype, SessionFactory factory) {
@@ -68,6 +70,7 @@ public SessionSlot(EventBus bus, Capabilities stereotype, SessionFactory factory
6870
this.factory = Require.nonNull("Session factory", factory);
6971
this.supportingCdp = isSlotSupportingCdp(this.stereotype);
7072
this.supportingBiDi = isSlotSupportingBiDi(this.stereotype);
73+
this.connectionCounter = new AtomicLong();
7174
}
7275

7376
public UUID getId() {
@@ -112,6 +115,7 @@ public void stop() {
112115
LOG.log(Level.WARNING, "Unable to cleanly close session", e);
113116
}
114117
currentSession = null;
118+
connectionCounter.set(0);
115119
release();
116120
bus.fire(new SessionClosedEvent(id));
117121
LOG.info(String.format("Stopping session %s", id));
@@ -148,6 +152,7 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
148152
if (possibleSession.isRight()) {
149153
ActiveSession session = possibleSession.right();
150154
currentSession = session;
155+
connectionCounter.set(0);
151156
return Either.right(session);
152157
} else {
153158
return Either.left(possibleSession.left());
@@ -185,4 +190,8 @@ public boolean hasRelayFactory() {
185190
public boolean isRelayServiceUp() {
186191
return hasRelayFactory() && ((RelaySessionFactory) factory).isServiceUp();
187192
}
193+
194+
public AtomicLong getConnectionCounter() {
195+
return connectionCounter;
196+
}
188197
}

java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java

+12
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,18 @@ public boolean isSessionOwner(SessionId id) {
174174
return Boolean.TRUE.equals(Values.get(res, Boolean.class));
175175
}
176176

177+
@Override
178+
public boolean tryAcquireConnection(SessionId id) {
179+
Require.nonNull("Session ID", id);
180+
181+
HttpRequest req = new HttpRequest(POST, "/se/grid/node/connection/" + id);
182+
HttpTracing.inject(tracer, tracer.getCurrentContext(), req);
183+
184+
HttpResponse res = client.with(addSecret).execute(req);
185+
186+
return Boolean.TRUE.equals(Values.get(res, Boolean.class));
187+
}
188+
177189
@Override
178190
public Session getSession(SessionId id) throws NoSuchSessionException {
179191
Require.nonNull("Session ID", id);

java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java

+5
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,11 @@ public boolean isSessionOwner(SessionId id) {
445445
return running != null && running.getId().equals(id);
446446
}
447447

448+
@Override
449+
public boolean tryAcquireConnection(SessionId id) {
450+
return false;
451+
}
452+
448453
@Override
449454
public boolean isSupporting(Capabilities capabilities) {
450455
return Objects.equals("cake", capabilities.getCapability("cheese"));

0 commit comments

Comments
 (0)