Skip to content

Commit d8cce8d

Browse files
VietND96VietND96
committed
[grid] Enhance smart max-sessions handling by calculating total from driver configurations
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent b731bb4 commit d8cce8d

File tree

4 files changed

+262
-26
lines changed

4 files changed

+262
-26
lines changed

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

Lines changed: 232 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -241,22 +241,27 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
241241
+ "Issues related to parallel testing with Internet Explored won't be accepted.");
242242
LOG.warning("Double check if enabling 'override-max-sessions' is really needed");
243243
}
244-
int maxSessions = getMaxSessions();
245-
if (maxSessions > DEFAULT_MAX_SESSIONS) {
246-
LOG.log(Level.WARNING, "Max sessions set to {0} ", maxSessions);
247-
}
244+
// Use node max-sessions for initial driver discovery
245+
int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS);
248246

249247
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers =
250-
discoverDrivers(maxSessions, factoryFactory);
248+
discoverDrivers(nodeMaxSessions, factoryFactory);
251249

252250
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories =
253251
ImmutableMultimap.builder();
254252

255253
addDriverFactoriesFromConfig(sessionFactories);
256-
addDriverConfigs(factoryFactory, sessionFactories, maxSessions);
254+
addDriverConfigs(factoryFactory, sessionFactories, nodeMaxSessions);
257255
addSpecificDrivers(allDrivers, sessionFactories);
258256
addDetectedDrivers(allDrivers, sessionFactories);
259257

258+
// Log final max sessions after all drivers are configured
259+
int finalMaxSessions = getMaxSessions();
260+
LOG.log(Level.INFO, "Node concurrent sessions: {0}", finalMaxSessions);
261+
if (finalMaxSessions > DEFAULT_MAX_SESSIONS) {
262+
LOG.log(Level.WARNING, "Max sessions set to {0} ", finalMaxSessions);
263+
}
264+
260265
return sessionFactories.build().asMap();
261266
}
262267

@@ -265,10 +270,197 @@ public int getMaxSessions() {
265270
Require.positive("Driver max sessions", maxSessions);
266271
boolean overrideMaxSessions =
267272
config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS);
268-
if (maxSessions > DEFAULT_MAX_SESSIONS && overrideMaxSessions) {
269-
return maxSessions;
273+
274+
// Always calculate sum of actual driver sessions for consistency
275+
int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers(maxSessions);
276+
277+
if (overrideMaxSessions) {
278+
return totalActualSessions;
279+
} else {
280+
// When override-max-sessions = false, return sum of actual sessions but cap at CPU cores
281+
return totalActualSessions > 0
282+
? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS)
283+
: Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
284+
}
285+
}
286+
287+
/**
288+
* Calculate the actual max-sessions per driver config based on the current configuration. This
289+
* method ensures consistency between getMaxSessions() and actual session allocation.
290+
*/
291+
private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
292+
Map<String, Integer> result = new HashMap<>();
293+
boolean overrideMaxSessions =
294+
config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS);
295+
int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS);
296+
297+
// Handle explicit driver configurations
298+
Optional<List<List<String>>> driverConfigs =
299+
config.getArray(NODE_SECTION, "driver-configuration");
300+
if (driverConfigs.isPresent()) {
301+
List<List<String>> drivers = driverConfigs.get();
302+
if (drivers.isEmpty()) {
303+
config.getAll(NODE_SECTION, "driver-configuration").ifPresent(drivers::add);
304+
}
305+
306+
List<Map<String, String>> configList = new ArrayList<>();
307+
for (List<String> driver : drivers) {
308+
Map<String, String> configMap = new HashMap<>();
309+
for (String setting : driver) {
310+
String[] values = setting.split("=", 2);
311+
if (values.length == 2) {
312+
configMap.put(values[0], unquote(values[1]));
313+
}
314+
}
315+
if (configMap.containsKey("display-name") && configMap.containsKey("stereotype")) {
316+
configList.add(configMap);
317+
}
318+
}
319+
320+
if (!configList.isEmpty()) {
321+
if (overrideMaxSessions) {
322+
// When override-max-sessions = true, use explicit values or node default
323+
for (Map<String, String> configMap : configList) {
324+
String displayName = configMap.get("display-name");
325+
int driverMaxSessions =
326+
Integer.parseInt(
327+
configMap.getOrDefault("max-sessions", String.valueOf(nodeMaxSessions)));
328+
result.put(displayName, driverMaxSessions);
329+
}
330+
} else {
331+
// When override-max-sessions = false, use CPU-based distribution with explicit overrides
332+
final int sessionsPerDriverConfig;
333+
if (configList.size() > DEFAULT_MAX_SESSIONS) {
334+
sessionsPerDriverConfig = 1;
335+
} else {
336+
sessionsPerDriverConfig = DEFAULT_MAX_SESSIONS / configList.size();
337+
}
338+
339+
for (Map<String, String> configMap : configList) {
340+
String displayName = configMap.get("display-name");
341+
int driverMaxSessions = sessionsPerDriverConfig;
342+
343+
// Check if driver config has explicit max-sessions within allowed range
344+
if (configMap.containsKey("max-sessions")) {
345+
int explicitMaxSessions = Integer.parseInt(configMap.get("max-sessions"));
346+
if (explicitMaxSessions >= 1 && explicitMaxSessions <= sessionsPerDriverConfig) {
347+
driverMaxSessions = explicitMaxSessions;
348+
}
349+
} else {
350+
// Only apply node max-sessions override if driver config doesn't have explicit
351+
// max-sessions
352+
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
353+
if (nodeMaxSessions >= 1 && nodeMaxSessions <= sessionsPerDriverConfig) {
354+
driverMaxSessions = nodeMaxSessions;
355+
}
356+
}
357+
}
358+
359+
result.put(displayName, driverMaxSessions);
360+
}
361+
}
362+
return result;
363+
}
364+
}
365+
366+
// Handle detected drivers if no explicit configs
367+
if (config.getBool(NODE_SECTION, "detect-drivers").orElse(DEFAULT_DETECT_DRIVERS)) {
368+
List<WebDriverInfo> detectedDrivers = getDetectedDrivers();
369+
if (!detectedDrivers.isEmpty()) {
370+
if (overrideMaxSessions) {
371+
// When override-max-sessions = true, each driver gets node max-sessions
372+
for (WebDriverInfo info : detectedDrivers) {
373+
if (info.getMaximumSimultaneousSessions() == 1
374+
&& SINGLE_SESSION_DRIVERS.contains(
375+
info.getDisplayName().toLowerCase(Locale.ENGLISH))) {
376+
result.put(info.getDisplayName(), 1);
377+
} else {
378+
result.put(info.getDisplayName(), nodeMaxSessions);
379+
}
380+
}
381+
} else {
382+
// When override-max-sessions = false, use optimized CPU distribution
383+
Map<WebDriverInfo, Integer> sessionsPerDriver =
384+
calculateOptimizedCpuDistribution(detectedDrivers);
385+
386+
// Check if node max-sessions is explicitly set and within allowed range
387+
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
388+
for (WebDriverInfo info : detectedDrivers) {
389+
int calculatedSessions = sessionsPerDriver.get(info);
390+
if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) {
391+
result.put(info.getDisplayName(), nodeMaxSessions);
392+
} else {
393+
result.put(info.getDisplayName(), calculatedSessions);
394+
}
395+
}
396+
} else {
397+
for (Map.Entry<WebDriverInfo, Integer> entry : sessionsPerDriver.entrySet()) {
398+
result.put(entry.getKey().getDisplayName(), entry.getValue());
399+
}
400+
}
401+
}
402+
}
403+
}
404+
405+
return result;
406+
}
407+
408+
private Map<WebDriverInfo, Integer> calculateOptimizedCpuDistribution(List<WebDriverInfo> infos) {
409+
Map<WebDriverInfo, Integer> sessionsPerDriver = new HashMap<>();
410+
411+
// First, allocate sessions for constrained drivers (like Safari)
412+
int remainingCores = DEFAULT_MAX_SESSIONS;
413+
List<WebDriverInfo> constrainedDrivers = new ArrayList<>();
414+
List<WebDriverInfo> flexibleDrivers = new ArrayList<>();
415+
416+
for (WebDriverInfo info : infos) {
417+
if (info.getMaximumSimultaneousSessions() == 1
418+
&& SINGLE_SESSION_DRIVERS.contains(info.getDisplayName().toLowerCase(Locale.ENGLISH))) {
419+
constrainedDrivers.add(info);
420+
sessionsPerDriver.put(info, 1);
421+
remainingCores--;
422+
} else {
423+
flexibleDrivers.add(info);
424+
}
270425
}
271-
return Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
426+
427+
// Then distribute remaining cores among flexible drivers
428+
if (flexibleDrivers.size() > 0 && remainingCores > 0) {
429+
int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size());
430+
for (WebDriverInfo info : flexibleDrivers) {
431+
sessionsPerDriver.put(info, sessionsPerFlexibleDriver);
432+
}
433+
} else if (flexibleDrivers.size() > 0) {
434+
// No remaining cores, give each flexible driver 1 session
435+
for (WebDriverInfo info : flexibleDrivers) {
436+
sessionsPerDriver.put(info, 1);
437+
}
438+
}
439+
440+
return sessionsPerDriver;
441+
}
442+
443+
private int calculateTotalMaxSessionsFromAllDrivers(int nodeMaxSessions) {
444+
Map<String, Integer> actualMaxSessions = calculateActualMaxSessionsPerDriverConfig();
445+
return actualMaxSessions.values().stream().mapToInt(Integer::intValue).sum();
446+
}
447+
448+
private List<WebDriverInfo> getDetectedDrivers() {
449+
List<WebDriverInfo> infos = new ArrayList<>();
450+
if (config.getBool(NODE_SECTION, "selenium-manager").orElse(DEFAULT_USE_SELENIUM_MANAGER)) {
451+
List<WebDriverInfo> driversSM =
452+
StreamSupport.stream(ServiceLoader.load(WebDriverInfo.class).spliterator(), false)
453+
.filter(WebDriverInfo::isAvailable)
454+
.collect(Collectors.toList());
455+
infos.addAll(driversSM);
456+
} else {
457+
List<WebDriverInfo> localDrivers =
458+
StreamSupport.stream(ServiceLoader.load(WebDriverInfo.class).spliterator(), false)
459+
.filter(WebDriverInfo::isPresent)
460+
.collect(Collectors.toList());
461+
infos.addAll(localDrivers);
462+
}
463+
return infos;
272464
}
273465

274466
public int getConnectionLimitPerSession() {
@@ -460,6 +652,14 @@ private void addDriverConfigs(
460652
List<WebDriverInfo> infoList = new ArrayList<>();
461653
ServiceLoader.load(WebDriverInfo.class).forEach(infoList::add);
462654

655+
// Get actual max-sessions per driver config from centralized calculation
656+
Map<String, Integer> actualMaxSessionsPerConfig =
657+
calculateActualMaxSessionsPerDriverConfig();
658+
boolean overrideMaxSessions =
659+
config
660+
.getBool(NODE_SECTION, "override-max-sessions")
661+
.orElse(OVERRIDE_MAX_SESSIONS);
662+
463663
// iterate over driver configs
464664
configList.forEach(
465665
thisConfig -> {
@@ -494,8 +694,6 @@ private void addDriverConfigs(
494694
}
495695

496696
Capabilities stereotype = enhanceStereotype(confStereotype);
497-
String configName =
498-
thisConfig.getOrDefault("display-name", "Custom Slot Config");
499697

500698
WebDriverInfo info =
501699
infoList.stream()
@@ -506,9 +704,11 @@ private void addDriverConfigs(
506704
new ConfigException(
507705
"Unable to find matching driver for %s", stereotype));
508706

707+
// Use the centralized calculation for consistency
708+
String configName =
709+
thisConfig.getOrDefault("display-name", "Custom Slot Config");
509710
int driverMaxSessions =
510-
Integer.parseInt(
511-
thisConfig.getOrDefault("max-sessions", String.valueOf(maxSessions)));
711+
actualMaxSessionsPerConfig.getOrDefault(configName, DEFAULT_MAX_SESSIONS);
512712
Require.positive("Driver max sessions", driverMaxSessions);
513713

514714
WebDriverInfo driverInfoConfig =
@@ -656,6 +856,15 @@ private Map<WebDriverInfo, Collection<SessionFactory>> discoverDrivers(
656856
List<DriverService.Builder<?, ?>> builders = new ArrayList<>();
657857
ServiceLoader.load(DriverService.Builder.class).forEach(builders::add);
658858

859+
// Get actual max-sessions per driver from centralized calculation
860+
Map<String, Integer> actualMaxSessionsPerConfig = calculateActualMaxSessionsPerDriverConfig();
861+
final Map<WebDriverInfo, Integer> sessionsPerDriver = new HashMap<>();
862+
for (WebDriverInfo info : infos) {
863+
int sessions =
864+
actualMaxSessionsPerConfig.getOrDefault(info.getDisplayName(), DEFAULT_MAX_SESSIONS);
865+
sessionsPerDriver.put(info, sessions);
866+
}
867+
659868
Multimap<WebDriverInfo, SessionFactory> toReturn = HashMultimap.create();
660869
infos.forEach(
661870
info -> {
@@ -666,7 +875,8 @@ private Map<WebDriverInfo, Collection<SessionFactory>> discoverDrivers(
666875
.ifPresent(
667876
builder -> {
668877
ImmutableCapabilities immutable = new ImmutableCapabilities(caps);
669-
int maxDriverSessions = getDriverMaxSessions(info, maxSessions);
878+
int driverMaxSessions = sessionsPerDriver.getOrDefault(info, 1);
879+
int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions);
670880
for (int i = 0; i < maxDriverSessions; i++) {
671881
toReturn.putAll(info, factoryFactory.apply(immutable));
672882
}
@@ -735,7 +945,14 @@ private int getDriverMaxSessions(WebDriverInfo info, int desiredMaxSessions) {
735945
}
736946
boolean overrideMaxSessions =
737947
config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS);
738-
if (desiredMaxSessions > info.getMaximumSimultaneousSessions() && overrideMaxSessions) {
948+
949+
if (!overrideMaxSessions) {
950+
// When override-max-sessions = false, use the calculated sessions per driver config
951+
return Math.min(info.getMaximumSimultaneousSessions(), desiredMaxSessions);
952+
}
953+
954+
// When override-max-sessions = true, respect the driver config max-sessions
955+
if (desiredMaxSessions > info.getMaximumSimultaneousSessions()) {
739956
String logMessage =
740957
String.format(
741958
"Overriding max recommended number of %s concurrent sessions for %s, setting it to"

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -600,22 +600,21 @@ void shouldNotOverrideMaxSessionsByDefault() {
600600
int overriddenMaxSessions = maxRecommendedSessions + 10;
601601
Config config =
602602
new MapConfig(singletonMap("node", ImmutableMap.of("max-sessions", overriddenMaxSessions)));
603+
604+
NodeOptions nodeOptions = new NodeOptions(config);
603605
List<Capabilities> reported = new ArrayList<>();
604606
try {
605-
new NodeOptions(config)
606-
.getSessionFactories(
607-
caps -> {
608-
reported.add(caps);
609-
return Collections.singleton(HelperFactory.create(config, caps));
610-
});
607+
nodeOptions.getSessionFactories(
608+
caps -> {
609+
reported.add(caps);
610+
return Collections.singleton(HelperFactory.create(config, caps));
611+
});
611612
} catch (ConfigException e) {
612613
// Fall through
613614
}
614-
long chromeSlots =
615-
reported.stream()
616-
.filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName()))
617-
.count();
618-
assertThat(chromeSlots).isEqualTo(maxRecommendedSessions);
615+
616+
// Verify node max-sessions is within CPU limit (not the overridden value)
617+
assertThat(nodeOptions.getMaxSessions()).isLessThanOrEqualTo(maxRecommendedSessions);
619618
}
620619

621620
@Test

‎py/selenium/webdriver/remote/server.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717

1818
import collections
19+
import multiprocessing
1920
import os
2021
import re
2122
import shutil
@@ -170,6 +171,10 @@ def start(self):
170171
"true",
171172
"--enable-managed-downloads",
172173
"true",
174+
"--override-max-sessions",
175+
"true",
176+
"--max-sessions",
177+
str(multiprocessing.cpu_count()),
173178
]
174179
if self.host is not None:
175180
command.extend(["--host", self.host])

0 commit comments

Comments
 (0)