Skip to content

Commit c39edad

Browse files
diemoldiemol
committed
[grid] Allowing users to overwrite recommended max sessions
Until now, we have had the limit per Node to run up to (# of available processors) concurrent sessions on each host, as the idea is to embrace stability and reliability to the running browser sessions, as one browser session should use one processor. Except IE and Safari, which only run one concurrent session per host. If a user sets `override-max-sessions` to `true`, the value for `max-sessions` can be higher than the recommended max sessions. A set of warnings are printed on the command line when this behaviour is detected. Fixes #9300
1 parent c078d31 commit c39edad

File tree

8 files changed

+130
-31
lines changed

8 files changed

+130
-31
lines changed

‎java/client/src/org/openqa/selenium/chromium/ChromiumDriverInfo.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ public abstract class ChromiumDriverInfo implements WebDriverInfo {
2323

2424
@Override
2525
public int getMaximumSimultaneousSessions() {
26-
return Runtime.getRuntime().availableProcessors() + 1;
26+
return Runtime.getRuntime().availableProcessors();
2727
}
2828
}

‎java/client/src/org/openqa/selenium/edge/EdgeDriverInfo.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.openqa.selenium.edge;
1818

1919
import com.google.auto.service.AutoService;
20+
2021
import org.openqa.selenium.Capabilities;
2122
import org.openqa.selenium.ImmutableCapabilities;
2223
import org.openqa.selenium.SessionNotCreatedException;
@@ -71,7 +72,7 @@ public boolean isAvailable() {
7172

7273
@Override
7374
public int getMaximumSimultaneousSessions() {
74-
return Runtime.getRuntime().availableProcessors() + 1;
75+
return Runtime.getRuntime().availableProcessors();
7576
}
7677

7778
@Override

‎java/client/src/org/openqa/selenium/firefox/GeckoDriverInfo.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.openqa.selenium.firefox;
1919

2020
import com.google.auto.service.AutoService;
21+
2122
import org.openqa.selenium.Capabilities;
2223
import org.openqa.selenium.ImmutableCapabilities;
2324
import org.openqa.selenium.SessionNotCreatedException;
@@ -77,7 +78,7 @@ public boolean isAvailable() {
7778

7879
@Override
7980
public int getMaximumSimultaneousSessions() {
80-
return Runtime.getRuntime().availableProcessors() + 1;
81+
return Runtime.getRuntime().availableProcessors();
8182
}
8283

8384
@Override

‎java/client/src/org/openqa/selenium/firefox/xpi/XpiDriverInfo.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.openqa.selenium.firefox.xpi;
1919

2020
import com.google.auto.service.AutoService;
21+
2122
import org.openqa.selenium.Capabilities;
2223
import org.openqa.selenium.ImmutableCapabilities;
2324
import org.openqa.selenium.SessionNotCreatedException;
@@ -72,7 +73,7 @@ public boolean isAvailable() {
7273

7374
@Override
7475
public int getMaximumSimultaneousSessions() {
75-
return Runtime.getRuntime().availableProcessors() + 1;
76+
return Runtime.getRuntime().availableProcessors();
7677
}
7778

7879
@Override

‎java/client/src/org/openqa/selenium/opera/OperaDriverInfo.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717

1818
package org.openqa.selenium.opera;
1919

20-
import static org.openqa.selenium.remote.CapabilityType.BROWSER_NAME;
21-
2220
import com.google.auto.service.AutoService;
2321

2422
import org.openqa.selenium.Capabilities;
@@ -31,6 +29,8 @@
3129

3230
import java.util.Optional;
3331

32+
import static org.openqa.selenium.remote.CapabilityType.BROWSER_NAME;
33+
3434
@AutoService(WebDriverInfo.class)
3535
public class OperaDriverInfo implements WebDriverInfo {
3636

@@ -67,7 +67,7 @@ public boolean isAvailable() {
6767

6868
@Override
6969
public int getMaximumSimultaneousSessions() {
70-
return Runtime.getRuntime().availableProcessors() + 1;
70+
return Runtime.getRuntime().availableProcessors();
7171
}
7272

7373
@Override

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,6 @@
1717

1818
package org.openqa.selenium.grid.node.config;
1919

20-
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
21-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
22-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_HEARTBEAT_PERIOD;
23-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_MAX_SESSIONS;
24-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_CYCLE;
25-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_PERIOD;
26-
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_SESSION_TIMEOUT;
27-
import static org.openqa.selenium.grid.node.config.NodeOptions.NODE_SECTION;
28-
2920
import com.google.auto.service.AutoService;
3021

3122
import com.beust.jcommander.Parameter;
@@ -40,6 +31,16 @@
4031
import java.util.List;
4132
import java.util.Set;
4233

34+
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
35+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
36+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_HEARTBEAT_PERIOD;
37+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_MAX_SESSIONS;
38+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_CYCLE;
39+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_PERIOD;
40+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_SESSION_TIMEOUT;
41+
import static org.openqa.selenium.grid.node.config.NodeOptions.NODE_SECTION;
42+
import static org.openqa.selenium.grid.node.config.NodeOptions.OVERRIDE_MAX_SESSIONS;
43+
4344
@SuppressWarnings("unused")
4445
@AutoService(HasRoles.class)
4546
public class NodeFlags implements HasRoles {
@@ -51,6 +52,16 @@ public class NodeFlags implements HasRoles {
5152
@ConfigValue(section = NODE_SECTION, name = "max-sessions", example = "8")
5253
public int maxSessions = DEFAULT_MAX_SESSIONS;
5354

55+
@Parameter(
56+
names = {"--override-max-sessions"},
57+
arity = 1,
58+
description = "The # of available processos is the recommended max sessions value (1 browser "
59+
+ "session per processor). Setting this flag to true allows the recommended max "
60+
+ "value to be overwritten. Session stability and reliability might suffer as "
61+
+ "the host could run out of resources.")
62+
@ConfigValue(section = NODE_SECTION, name = "override-max-sessions", example = "false")
63+
public Boolean overrideMaxSessions = OVERRIDE_MAX_SESSIONS;
64+
5465
@Parameter(
5566
names = {"--session-timeout"},
5667
description = "Let X be the session-timeout in seconds. The Node will automatically kill "

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

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public class NodeOptions {
6363
public static final int DEFAULT_SESSION_TIMEOUT = 300;
6464
static final String NODE_SECTION = "node";
6565
static final boolean DEFAULT_DETECT_DRIVERS = true;
66+
static final boolean OVERRIDE_MAX_SESSIONS = false;
6667
static final int DEFAULT_REGISTER_CYCLE = 10;
6768
static final int DEFAULT_REGISTER_PERIOD = 120;
6869

@@ -72,7 +73,6 @@ public class NodeOptions {
7273
private static final ImmutableCapabilities CURRENT_PLATFORM =
7374
new ImmutableCapabilities("platformName", Platform.getCurrent());
7475

75-
7676
private final Config config;
7777

7878
public NodeOptions(Config config) {
@@ -121,7 +121,18 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
121121
/* Danger! Java stereotype ahead! */
122122
Function<Capabilities, Collection<SessionFactory>> factoryFactory) {
123123

124+
LOG.log(Level.INFO, "Detected {0} available processors", DEFAULT_MAX_SESSIONS);
124125
int maxSessions = getMaxSessions();
126+
if (maxSessions > DEFAULT_MAX_SESSIONS) {
127+
LOG.log(Level.WARNING,
128+
"Overriding max recommended number of {0} concurrent sessions. "
129+
+ "Session stability and reliability might suffer!",
130+
DEFAULT_MAX_SESSIONS);
131+
LOG.warning("One browser session is recommended per available processor. IE and "
132+
+ "Safari are always limited to 1 session per host.");
133+
LOG.warning("Double check if enabling 'override-max-sessions' is really needed");
134+
LOG.log(Level.WARNING, "Max sessions set to {0} ", maxSessions);
135+
}
125136

126137
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers =
127138
discoverDrivers(maxSessions, factoryFactory);
@@ -138,9 +149,15 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
138149
}
139150

140151
public int getMaxSessions() {
141-
return Math.min(
142-
config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS),
143-
DEFAULT_MAX_SESSIONS);
152+
int maxSessions = config.getInt(NODE_SECTION, "max-sessions")
153+
.orElse(DEFAULT_MAX_SESSIONS);
154+
Require.positive("Driver max sessions", maxSessions);
155+
boolean overrideMaxSessions = config.getBool(NODE_SECTION, "override-max-sessions")
156+
.orElse(OVERRIDE_MAX_SESSIONS);
157+
if (maxSessions > DEFAULT_MAX_SESSIONS && overrideMaxSessions) {
158+
return maxSessions;
159+
}
160+
return Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
144161
}
145162

146163
public Duration getSessionTimeout() {
@@ -259,8 +276,8 @@ private void addDriverConfigs(
259276
builders.stream()
260277
.filter(builder -> builder.score(stereotype) > 0)
261278
.forEach(builder -> {
262-
int maxSessions = Math.min(info.getMaximumSimultaneousSessions(), driverMaxSessions);
263-
for (int i = 0; i < maxSessions; i++) {
279+
int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions);
280+
for (int i = 0; i < maxDriverSessions; i++) {
264281
driverConfigs.putAll(driverInfoConfig, factoryFactory.apply(stereotype));
265282
}
266283
});
@@ -376,7 +393,8 @@ private Map<WebDriverInfo, Collection<SessionFactory>> discoverDrivers(
376393
builders.stream()
377394
.filter(builder -> builder.score(caps) > 0)
378395
.forEach(builder -> {
379-
for (int i = 0; i < Math.min(info.getMaximumSimultaneousSessions(), maxSessions); i++) {
396+
int maxDriverSessions = getDriverMaxSessions(info, maxSessions);
397+
for (int i = 0; i < maxDriverSessions; i++) {
380398
toReturn.putAll(info, factoryFactory.apply(caps));
381399
}
382400
});
@@ -426,6 +444,23 @@ public Optional<WebDriver> createDriver(Capabilities capabilities)
426444
};
427445
}
428446

447+
private int getDriverMaxSessions(WebDriverInfo info, int desiredMaxSessions) {
448+
// IE and Safari
449+
if (info.getMaximumSimultaneousSessions() == 1) {
450+
return info.getMaximumSimultaneousSessions();
451+
}
452+
if (desiredMaxSessions > info.getMaximumSimultaneousSessions()) {
453+
String logMessage = String.format(
454+
"Overriding max recommended number of %s concurrent sessions for %s, setting it to %s",
455+
info.getMaximumSimultaneousSessions(),
456+
info.getDisplayName(),
457+
desiredMaxSessions);
458+
LOG.log(Level.WARNING, logMessage);
459+
return desiredMaxSessions;
460+
}
461+
return Math.min(info.getMaximumSimultaneousSessions(), desiredMaxSessions);
462+
}
463+
429464
private void report(Map.Entry<WebDriverInfo, Collection<SessionFactory>> entry) {
430465
StringBuilder caps = new StringBuilder();
431466
try (JsonOutput out = JSON.newOutput(caps)) {

‎java/server/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@
1717

1818
package org.openqa.selenium.grid.node.config;
1919

20-
import static java.util.Collections.emptyMap;
21-
import static java.util.Collections.emptySet;
22-
import static java.util.Collections.singletonMap;
23-
import static org.assertj.core.api.Assertions.assertThat;
24-
import static org.junit.Assert.fail;
25-
import static org.junit.Assume.assumeFalse;
26-
import static org.junit.Assume.assumeTrue;
27-
2820
import com.google.common.collect.ImmutableMap;
2921

3022
import org.assertj.core.api.Condition;
@@ -54,6 +46,14 @@
5446
import java.util.List;
5547
import java.util.Map;
5648

49+
import static java.util.Collections.emptyMap;
50+
import static java.util.Collections.emptySet;
51+
import static java.util.Collections.singletonMap;
52+
import static org.assertj.core.api.Assertions.assertThat;
53+
import static org.junit.Assert.fail;
54+
import static org.junit.Assume.assumeFalse;
55+
import static org.junit.Assume.assumeTrue;
56+
5757
@SuppressWarnings("DuplicatedCode")
5858
public class NodeOptionsTest {
5959

@@ -282,6 +282,55 @@ public void driversConfigNeedsStereotypeField() {
282282
assertThat(reported).isEmpty();
283283
}
284284

285+
@Test
286+
public void shouldNotOverrideMaxSessionsByDefault() {
287+
assumeTrue("ChromeDriver needs to be available", new ChromeDriverInfo().isAvailable());
288+
int maxRecommendedSessions = Runtime.getRuntime().availableProcessors();
289+
int overriddenMaxSessions = maxRecommendedSessions + 10;
290+
Config config = new MapConfig(
291+
singletonMap("node",
292+
ImmutableMap
293+
.of("max-sessions", overriddenMaxSessions)));
294+
List<Capabilities> reported = new ArrayList<>();
295+
try {
296+
new NodeOptions(config).getSessionFactories(caps -> {
297+
reported.add(caps);
298+
return Collections.singleton(HelperFactory.create(config, caps));
299+
});
300+
} catch (ConfigException e) {
301+
// Fall through
302+
}
303+
long chromeSlots = reported.stream()
304+
.filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName()))
305+
.count();
306+
assertThat(chromeSlots).isEqualTo(maxRecommendedSessions);
307+
}
308+
309+
@Test
310+
public void canOverrideMaxSessionsWithFlag() {
311+
assumeTrue("ChromeDriver needs to be available", new ChromeDriverInfo().isAvailable());
312+
int maxRecommendedSessions = Runtime.getRuntime().availableProcessors();
313+
int overriddenMaxSessions = maxRecommendedSessions + 10;
314+
Config config = new MapConfig(
315+
singletonMap("node",
316+
ImmutableMap
317+
.of("max-sessions", overriddenMaxSessions,
318+
"override-max-sessions", true)));
319+
List<Capabilities> reported = new ArrayList<>();
320+
try {
321+
new NodeOptions(config).getSessionFactories(caps -> {
322+
reported.add(caps);
323+
return Collections.singleton(HelperFactory.create(config, caps));
324+
});
325+
} catch (ConfigException e) {
326+
// Fall through
327+
}
328+
long chromeSlots = reported.stream()
329+
.filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName()))
330+
.count();
331+
assertThat(chromeSlots).isEqualTo(overriddenMaxSessions);
332+
}
333+
285334
private Condition<? super List<? extends Capabilities>> supporting(String name) {
286335
return new Condition<>(
287336
caps -> caps.stream().anyMatch(cap -> name.equals(cap.getBrowserName())),
@@ -290,6 +339,7 @@ private Condition<? super List<? extends Capabilities>> supporting(String name)
290339
}
291340

292341
public static class HelperFactory {
342+
293343
public static SessionFactory create(Config config, Capabilities caps) {
294344
return new SessionFactory() {
295345
@Override

0 commit comments

Comments
 (0)