From d8cce8ddce71728f13daac7c7b98e367dba3c3bb Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 22 Sep 2025 00:23:48 +0700 Subject: [PATCH 1/6] [grid] Enhance smart max-sessions handling by calculating total from driver configurations Signed-off-by: Viet Nguyen Duc --- .../grid/node/config/NodeOptions.java | 247 ++++++++++++++++-- .../grid/node/config/NodeOptionsTest.java | 21 +- py/selenium/webdriver/remote/server.py | 5 + rb/lib/selenium/server.rb | 15 ++ 4 files changed, 262 insertions(+), 26 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index 5afc047497960..6a730dd2d6347 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -241,22 +241,27 @@ public Map> getSessionFactories( + "Issues related to parallel testing with Internet Explored won't be accepted."); LOG.warning("Double check if enabling 'override-max-sessions' is really needed"); } - int maxSessions = getMaxSessions(); - if (maxSessions > DEFAULT_MAX_SESSIONS) { - LOG.log(Level.WARNING, "Max sessions set to {0} ", maxSessions); - } + // Use node max-sessions for initial driver discovery + int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS); Map> allDrivers = - discoverDrivers(maxSessions, factoryFactory); + discoverDrivers(nodeMaxSessions, factoryFactory); ImmutableMultimap.Builder sessionFactories = ImmutableMultimap.builder(); addDriverFactoriesFromConfig(sessionFactories); - addDriverConfigs(factoryFactory, sessionFactories, maxSessions); + addDriverConfigs(factoryFactory, sessionFactories, nodeMaxSessions); addSpecificDrivers(allDrivers, sessionFactories); addDetectedDrivers(allDrivers, sessionFactories); + // Log final max sessions after all drivers are configured + int finalMaxSessions = getMaxSessions(); + LOG.log(Level.INFO, "Node concurrent sessions: {0}", finalMaxSessions); + if (finalMaxSessions > DEFAULT_MAX_SESSIONS) { + LOG.log(Level.WARNING, "Max sessions set to {0} ", finalMaxSessions); + } + return sessionFactories.build().asMap(); } @@ -265,10 +270,197 @@ public int getMaxSessions() { Require.positive("Driver max sessions", maxSessions); boolean overrideMaxSessions = config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS); - if (maxSessions > DEFAULT_MAX_SESSIONS && overrideMaxSessions) { - return maxSessions; + + // Always calculate sum of actual driver sessions for consistency + int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers(maxSessions); + + if (overrideMaxSessions) { + return totalActualSessions; + } else { + // When override-max-sessions = false, return sum of actual sessions but cap at CPU cores + return totalActualSessions > 0 + ? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS) + : Math.min(maxSessions, DEFAULT_MAX_SESSIONS); + } + } + + /** + * Calculate the actual max-sessions per driver config based on the current configuration. This + * method ensures consistency between getMaxSessions() and actual session allocation. + */ + private Map calculateActualMaxSessionsPerDriverConfig() { + Map result = new HashMap<>(); + boolean overrideMaxSessions = + config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS); + int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS); + + // Handle explicit driver configurations + Optional>> driverConfigs = + config.getArray(NODE_SECTION, "driver-configuration"); + if (driverConfigs.isPresent()) { + List> drivers = driverConfigs.get(); + if (drivers.isEmpty()) { + config.getAll(NODE_SECTION, "driver-configuration").ifPresent(drivers::add); + } + + List> configList = new ArrayList<>(); + for (List driver : drivers) { + Map configMap = new HashMap<>(); + for (String setting : driver) { + String[] values = setting.split("=", 2); + if (values.length == 2) { + configMap.put(values[0], unquote(values[1])); + } + } + if (configMap.containsKey("display-name") && configMap.containsKey("stereotype")) { + configList.add(configMap); + } + } + + if (!configList.isEmpty()) { + if (overrideMaxSessions) { + // When override-max-sessions = true, use explicit values or node default + for (Map configMap : configList) { + String displayName = configMap.get("display-name"); + int driverMaxSessions = + Integer.parseInt( + configMap.getOrDefault("max-sessions", String.valueOf(nodeMaxSessions))); + result.put(displayName, driverMaxSessions); + } + } else { + // When override-max-sessions = false, use CPU-based distribution with explicit overrides + final int sessionsPerDriverConfig; + if (configList.size() > DEFAULT_MAX_SESSIONS) { + sessionsPerDriverConfig = 1; + } else { + sessionsPerDriverConfig = DEFAULT_MAX_SESSIONS / configList.size(); + } + + for (Map configMap : configList) { + String displayName = configMap.get("display-name"); + int driverMaxSessions = sessionsPerDriverConfig; + + // Check if driver config has explicit max-sessions within allowed range + if (configMap.containsKey("max-sessions")) { + int explicitMaxSessions = Integer.parseInt(configMap.get("max-sessions")); + if (explicitMaxSessions >= 1 && explicitMaxSessions <= sessionsPerDriverConfig) { + driverMaxSessions = explicitMaxSessions; + } + } else { + // Only apply node max-sessions override if driver config doesn't have explicit + // max-sessions + if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) { + if (nodeMaxSessions >= 1 && nodeMaxSessions <= sessionsPerDriverConfig) { + driverMaxSessions = nodeMaxSessions; + } + } + } + + result.put(displayName, driverMaxSessions); + } + } + return result; + } + } + + // Handle detected drivers if no explicit configs + if (config.getBool(NODE_SECTION, "detect-drivers").orElse(DEFAULT_DETECT_DRIVERS)) { + List detectedDrivers = getDetectedDrivers(); + if (!detectedDrivers.isEmpty()) { + if (overrideMaxSessions) { + // When override-max-sessions = true, each driver gets node max-sessions + for (WebDriverInfo info : detectedDrivers) { + if (info.getMaximumSimultaneousSessions() == 1 + && SINGLE_SESSION_DRIVERS.contains( + info.getDisplayName().toLowerCase(Locale.ENGLISH))) { + result.put(info.getDisplayName(), 1); + } else { + result.put(info.getDisplayName(), nodeMaxSessions); + } + } + } else { + // When override-max-sessions = false, use optimized CPU distribution + Map sessionsPerDriver = + calculateOptimizedCpuDistribution(detectedDrivers); + + // Check if node max-sessions is explicitly set and within allowed range + if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) { + for (WebDriverInfo info : detectedDrivers) { + int calculatedSessions = sessionsPerDriver.get(info); + if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) { + result.put(info.getDisplayName(), nodeMaxSessions); + } else { + result.put(info.getDisplayName(), calculatedSessions); + } + } + } else { + for (Map.Entry entry : sessionsPerDriver.entrySet()) { + result.put(entry.getKey().getDisplayName(), entry.getValue()); + } + } + } + } + } + + return result; + } + + private Map calculateOptimizedCpuDistribution(List infos) { + Map sessionsPerDriver = new HashMap<>(); + + // First, allocate sessions for constrained drivers (like Safari) + int remainingCores = DEFAULT_MAX_SESSIONS; + List constrainedDrivers = new ArrayList<>(); + List flexibleDrivers = new ArrayList<>(); + + for (WebDriverInfo info : infos) { + if (info.getMaximumSimultaneousSessions() == 1 + && SINGLE_SESSION_DRIVERS.contains(info.getDisplayName().toLowerCase(Locale.ENGLISH))) { + constrainedDrivers.add(info); + sessionsPerDriver.put(info, 1); + remainingCores--; + } else { + flexibleDrivers.add(info); + } } - return Math.min(maxSessions, DEFAULT_MAX_SESSIONS); + + // Then distribute remaining cores among flexible drivers + if (flexibleDrivers.size() > 0 && remainingCores > 0) { + int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size()); + for (WebDriverInfo info : flexibleDrivers) { + sessionsPerDriver.put(info, sessionsPerFlexibleDriver); + } + } else if (flexibleDrivers.size() > 0) { + // No remaining cores, give each flexible driver 1 session + for (WebDriverInfo info : flexibleDrivers) { + sessionsPerDriver.put(info, 1); + } + } + + return sessionsPerDriver; + } + + private int calculateTotalMaxSessionsFromAllDrivers(int nodeMaxSessions) { + Map actualMaxSessions = calculateActualMaxSessionsPerDriverConfig(); + return actualMaxSessions.values().stream().mapToInt(Integer::intValue).sum(); + } + + private List getDetectedDrivers() { + List infos = new ArrayList<>(); + if (config.getBool(NODE_SECTION, "selenium-manager").orElse(DEFAULT_USE_SELENIUM_MANAGER)) { + List driversSM = + StreamSupport.stream(ServiceLoader.load(WebDriverInfo.class).spliterator(), false) + .filter(WebDriverInfo::isAvailable) + .collect(Collectors.toList()); + infos.addAll(driversSM); + } else { + List localDrivers = + StreamSupport.stream(ServiceLoader.load(WebDriverInfo.class).spliterator(), false) + .filter(WebDriverInfo::isPresent) + .collect(Collectors.toList()); + infos.addAll(localDrivers); + } + return infos; } public int getConnectionLimitPerSession() { @@ -460,6 +652,14 @@ private void addDriverConfigs( List infoList = new ArrayList<>(); ServiceLoader.load(WebDriverInfo.class).forEach(infoList::add); + // Get actual max-sessions per driver config from centralized calculation + Map actualMaxSessionsPerConfig = + calculateActualMaxSessionsPerDriverConfig(); + boolean overrideMaxSessions = + config + .getBool(NODE_SECTION, "override-max-sessions") + .orElse(OVERRIDE_MAX_SESSIONS); + // iterate over driver configs configList.forEach( thisConfig -> { @@ -494,8 +694,6 @@ private void addDriverConfigs( } Capabilities stereotype = enhanceStereotype(confStereotype); - String configName = - thisConfig.getOrDefault("display-name", "Custom Slot Config"); WebDriverInfo info = infoList.stream() @@ -506,9 +704,11 @@ private void addDriverConfigs( new ConfigException( "Unable to find matching driver for %s", stereotype)); + // Use the centralized calculation for consistency + String configName = + thisConfig.getOrDefault("display-name", "Custom Slot Config"); int driverMaxSessions = - Integer.parseInt( - thisConfig.getOrDefault("max-sessions", String.valueOf(maxSessions))); + actualMaxSessionsPerConfig.getOrDefault(configName, DEFAULT_MAX_SESSIONS); Require.positive("Driver max sessions", driverMaxSessions); WebDriverInfo driverInfoConfig = @@ -656,6 +856,15 @@ private Map> discoverDrivers( List> builders = new ArrayList<>(); ServiceLoader.load(DriverService.Builder.class).forEach(builders::add); + // Get actual max-sessions per driver from centralized calculation + Map actualMaxSessionsPerConfig = calculateActualMaxSessionsPerDriverConfig(); + final Map sessionsPerDriver = new HashMap<>(); + for (WebDriverInfo info : infos) { + int sessions = + actualMaxSessionsPerConfig.getOrDefault(info.getDisplayName(), DEFAULT_MAX_SESSIONS); + sessionsPerDriver.put(info, sessions); + } + Multimap toReturn = HashMultimap.create(); infos.forEach( info -> { @@ -666,7 +875,8 @@ private Map> discoverDrivers( .ifPresent( builder -> { ImmutableCapabilities immutable = new ImmutableCapabilities(caps); - int maxDriverSessions = getDriverMaxSessions(info, maxSessions); + int driverMaxSessions = sessionsPerDriver.getOrDefault(info, 1); + int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions); for (int i = 0; i < maxDriverSessions; i++) { toReturn.putAll(info, factoryFactory.apply(immutable)); } @@ -735,7 +945,14 @@ private int getDriverMaxSessions(WebDriverInfo info, int desiredMaxSessions) { } boolean overrideMaxSessions = config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS); - if (desiredMaxSessions > info.getMaximumSimultaneousSessions() && overrideMaxSessions) { + + if (!overrideMaxSessions) { + // When override-max-sessions = false, use the calculated sessions per driver config + return Math.min(info.getMaximumSimultaneousSessions(), desiredMaxSessions); + } + + // When override-max-sessions = true, respect the driver config max-sessions + if (desiredMaxSessions > info.getMaximumSimultaneousSessions()) { String logMessage = String.format( "Overriding max recommended number of %s concurrent sessions for %s, setting it to" diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java index babd6692aac39..8b4ab87037428 100644 --- a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java @@ -600,22 +600,21 @@ void shouldNotOverrideMaxSessionsByDefault() { int overriddenMaxSessions = maxRecommendedSessions + 10; Config config = new MapConfig(singletonMap("node", ImmutableMap.of("max-sessions", overriddenMaxSessions))); + + NodeOptions nodeOptions = new NodeOptions(config); List reported = new ArrayList<>(); try { - new NodeOptions(config) - .getSessionFactories( - caps -> { - reported.add(caps); - return Collections.singleton(HelperFactory.create(config, caps)); - }); + nodeOptions.getSessionFactories( + caps -> { + reported.add(caps); + return Collections.singleton(HelperFactory.create(config, caps)); + }); } catch (ConfigException e) { // Fall through } - long chromeSlots = - reported.stream() - .filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName())) - .count(); - assertThat(chromeSlots).isEqualTo(maxRecommendedSessions); + + // Verify node max-sessions is within CPU limit (not the overridden value) + assertThat(nodeOptions.getMaxSessions()).isLessThanOrEqualTo(maxRecommendedSessions); } @Test diff --git a/py/selenium/webdriver/remote/server.py b/py/selenium/webdriver/remote/server.py index 09e9e647331fe..d461d100da4a2 100644 --- a/py/selenium/webdriver/remote/server.py +++ b/py/selenium/webdriver/remote/server.py @@ -16,6 +16,7 @@ # under the License. import collections +import multiprocessing import os import re import shutil @@ -170,6 +171,10 @@ def start(self): "true", "--enable-managed-downloads", "true", + "--override-max-sessions", + "true", + "--max-sessions", + str(multiprocessing.cpu_count()), ] if self.host is not None: command.extend(["--host", self.host]) diff --git a/rb/lib/selenium/server.rb b/rb/lib/selenium/server.rb index ef2b05e99f199..342274c3e227b 100644 --- a/rb/lib/selenium/server.rb +++ b/rb/lib/selenium/server.rb @@ -21,6 +21,7 @@ require 'selenium/webdriver/common/port_prober' require 'selenium/webdriver/common/socket_poller' require 'net/http' +require 'etc' module Selenium # @@ -177,6 +178,8 @@ def download_server(uri, destination) # @option opts [true,false] :background Run the server in the background (default: false) # @option opts [true,false,String] :log Either a path to a log file, # or true to pass server log to stdout. + # @option opts [true,false] :override_max_sessions Override the maximum number of sessions + # @option opts [Integer] :max_sessions Maximum number of concurrent sessions # @raise [Errno::ENOENT] if the jar file does not exist # @@ -198,6 +201,18 @@ def initialize(jar, opts = {}) @additional_args << opts[:log_level].to_s end + override_max_sessions = opts.fetch(:override_max_sessions, true) + if override_max_sessions + @additional_args << '--override-max-sessions' + @additional_args << override_max_sessions.to_s + end + + max_sessions = opts.fetch(:max_sessions, Etc.nprocessors) + if max_sessions + @additional_args << '--max-sessions' + @additional_args << max_sessions.to_s + end + @log_file = nil end From a030566f86e06b46f28e718a85188615c86946f1 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 22 Sep 2025 04:15:37 +0700 Subject: [PATCH 2/6] Fix review comments Signed-off-by: Viet Nguyen Duc --- .../grid/node/config/NodeOptions.java | 76 +++++++++++++++++-- rb/spec/unit/selenium/server_spec.rb | 3 + 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index 6a730dd2d6347..d8d067ebcfdd1 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -284,6 +284,43 @@ public int getMaxSessions() { } } + /** + * Safely parse an integer value for max-sessions with validation and logging. Guards against + * NumberFormatException and ensures the value is positive. + * + * @param value the string value to parse + * @param defaultValue the default value to use if parsing fails or value is invalid + * @param context descriptive context for logging (e.g., "driver config", "node config") + * @return a valid positive integer, or the default value if parsing fails + */ + private static int parseMaxSessionsSafely(String value, int defaultValue, String context) { + if (value == null || value.trim().isEmpty()) { + LOG.log( + Level.WARNING, + "Max-sessions value is null or empty for {0}, using default: {1}", + new Object[] {context, defaultValue}); + return defaultValue; + } + + try { + int parsedValue = Integer.parseInt(value.trim()); + if (parsedValue <= 0) { + LOG.log( + Level.WARNING, + "Max-sessions value {0} is not positive for {1}, using default: {2}", + new Object[] {parsedValue, context, defaultValue}); + return defaultValue; + } + return parsedValue; + } catch (NumberFormatException e) { + LOG.log( + Level.WARNING, + "Invalid max-sessions value ''{0}'' for {1}, using default: {2}. Error: {3}", + new Object[] {value, context, defaultValue, e.getMessage()}); + return defaultValue; + } + } + /** * Calculate the actual max-sessions per driver config based on the current configuration. This * method ensures consistency between getMaxSessions() and actual session allocation. @@ -323,8 +360,10 @@ private Map calculateActualMaxSessionsPerDriverConfig() { for (Map configMap : configList) { String displayName = configMap.get("display-name"); int driverMaxSessions = - Integer.parseInt( - configMap.getOrDefault("max-sessions", String.valueOf(nodeMaxSessions))); + parseMaxSessionsSafely( + configMap.get("max-sessions"), + nodeMaxSessions, + "driver config '" + displayName + "'"); result.put(displayName, driverMaxSessions); } } else { @@ -342,7 +381,11 @@ private Map calculateActualMaxSessionsPerDriverConfig() { // Check if driver config has explicit max-sessions within allowed range if (configMap.containsKey("max-sessions")) { - int explicitMaxSessions = Integer.parseInt(configMap.get("max-sessions")); + int explicitMaxSessions = + parseMaxSessionsSafely( + configMap.get("max-sessions"), + sessionsPerDriverConfig, + "driver config '" + displayName + "' explicit max-sessions"); if (explicitMaxSessions >= 1 && explicitMaxSessions <= sessionsPerDriverConfig) { driverMaxSessions = explicitMaxSessions; } @@ -427,14 +470,37 @@ private Map calculateOptimizedCpuDistribution(List 0 && remainingCores > 0) { int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size()); - for (WebDriverInfo info : flexibleDrivers) { - sessionsPerDriver.put(info, sessionsPerFlexibleDriver); + int remainderCores = remainingCores % flexibleDrivers.size(); + + // Distribute base sessions to all flexible drivers + for (int i = 0; i < flexibleDrivers.size(); i++) { + WebDriverInfo info = flexibleDrivers.get(i); + int sessions = sessionsPerFlexibleDriver; + + // Distribute remainder cores to the first 'remainderCores' drivers + if (i < remainderCores) { + sessions++; + } + + sessionsPerDriver.put(info, sessions); } + + LOG.log( + Level.FINE, + "Distributed {0} cores among {1} flexible drivers: {2} base sessions each, " + + "{3} drivers get +1 extra session", + new Object[] { + remainingCores, flexibleDrivers.size(), sessionsPerFlexibleDriver, remainderCores + }); } else if (flexibleDrivers.size() > 0) { // No remaining cores, give each flexible driver 1 session for (WebDriverInfo info : flexibleDrivers) { sessionsPerDriver.put(info, 1); } + LOG.log( + Level.FINE, + "No remaining cores available, assigning 1 session to each of {0} flexible drivers", + flexibleDrivers.size()); } return sessionsPerDriver; diff --git a/rb/spec/unit/selenium/server_spec.rb b/rb/spec/unit/selenium/server_spec.rb index eedd755dd4131..5c3520d8d057c 100644 --- a/rb/spec/unit/selenium/server_spec.rb +++ b/rb/spec/unit/selenium/server_spec.rb @@ -18,6 +18,7 @@ # under the License. require File.expand_path('webdriver/spec_helper', __dir__) +require 'etc' require 'selenium/server' module Selenium @@ -122,6 +123,8 @@ module Selenium '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', + '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') end From 5c5033c2241a2f4d3d92535a54a6e14c859366ee Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 22 Sep 2025 06:12:13 +0700 Subject: [PATCH 3/6] Fix tests Signed-off-by: Viet Nguyen Duc --- .../grid/node/config/NodeOptions.java | 97 ++++++++----------- .../grid/node/config/NodeOptionsTest.java | 2 +- .../selenium/grid/router/StressTest.java | 31 +++--- rb/spec/unit/selenium/server_spec.rb | 34 +++++-- 4 files changed, 81 insertions(+), 83 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index d8d067ebcfdd1..568ff09407b66 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -241,17 +241,13 @@ public Map> getSessionFactories( + "Issues related to parallel testing with Internet Explored won't be accepted."); LOG.warning("Double check if enabling 'override-max-sessions' is really needed"); } - // Use node max-sessions for initial driver discovery - int nodeMaxSessions = config.getInt(NODE_SECTION, "max-sessions").orElse(DEFAULT_MAX_SESSIONS); - - Map> allDrivers = - discoverDrivers(nodeMaxSessions, factoryFactory); + Map> allDrivers = discoverDrivers(factoryFactory); ImmutableMultimap.Builder sessionFactories = ImmutableMultimap.builder(); addDriverFactoriesFromConfig(sessionFactories); - addDriverConfigs(factoryFactory, sessionFactories, nodeMaxSessions); + addDriverConfigs(factoryFactory, sessionFactories); addSpecificDrivers(allDrivers, sessionFactories); addDetectedDrivers(allDrivers, sessionFactories); @@ -272,7 +268,7 @@ public int getMaxSessions() { config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS); // Always calculate sum of actual driver sessions for consistency - int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers(maxSessions); + int totalActualSessions = calculateTotalMaxSessionsFromAllDrivers(); if (overrideMaxSessions) { return totalActualSessions; @@ -367,33 +363,32 @@ private Map calculateActualMaxSessionsPerDriverConfig() { result.put(displayName, driverMaxSessions); } } else { - // When override-max-sessions = false, use CPU-based distribution with explicit overrides - final int sessionsPerDriverConfig; - if (configList.size() > DEFAULT_MAX_SESSIONS) { - sessionsPerDriverConfig = 1; - } else { - sessionsPerDriverConfig = DEFAULT_MAX_SESSIONS / configList.size(); - } + // When override-max-sessions = false, use optimized CPU distribution + List driverNames = + configList.stream() + .map(config -> config.get("display-name")) + .collect(Collectors.toList()); + Map sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames); for (Map configMap : configList) { String displayName = configMap.get("display-name"); - int driverMaxSessions = sessionsPerDriverConfig; + int driverMaxSessions = sessionsPerDriver.getOrDefault(displayName, 1); // Check if driver config has explicit max-sessions within allowed range if (configMap.containsKey("max-sessions")) { int explicitMaxSessions = parseMaxSessionsSafely( configMap.get("max-sessions"), - sessionsPerDriverConfig, + driverMaxSessions, "driver config '" + displayName + "' explicit max-sessions"); - if (explicitMaxSessions >= 1 && explicitMaxSessions <= sessionsPerDriverConfig) { + if (explicitMaxSessions >= 1 && explicitMaxSessions <= driverMaxSessions) { driverMaxSessions = explicitMaxSessions; } } else { // Only apply node max-sessions override if driver config doesn't have explicit // max-sessions if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) { - if (nodeMaxSessions >= 1 && nodeMaxSessions <= sessionsPerDriverConfig) { + if (nodeMaxSessions >= 1 && nodeMaxSessions <= driverMaxSessions) { driverMaxSessions = nodeMaxSessions; } } @@ -423,13 +418,16 @@ private Map calculateActualMaxSessionsPerDriverConfig() { } } else { // When override-max-sessions = false, use optimized CPU distribution - Map sessionsPerDriver = - calculateOptimizedCpuDistribution(detectedDrivers); + List driverNames = + detectedDrivers.stream() + .map(WebDriverInfo::getDisplayName) + .collect(Collectors.toList()); + Map sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames); // Check if node max-sessions is explicitly set and within allowed range if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) { for (WebDriverInfo info : detectedDrivers) { - int calculatedSessions = sessionsPerDriver.get(info); + int calculatedSessions = sessionsPerDriver.get(info.getDisplayName()); if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) { result.put(info.getDisplayName(), nodeMaxSessions); } else { @@ -437,8 +435,8 @@ private Map calculateActualMaxSessionsPerDriverConfig() { } } } else { - for (Map.Entry entry : sessionsPerDriver.entrySet()) { - result.put(entry.getKey().getDisplayName(), entry.getValue()); + for (Map.Entry entry : sessionsPerDriver.entrySet()) { + result.put(entry.getKey(), entry.getValue()); } } } @@ -448,33 +446,31 @@ private Map calculateActualMaxSessionsPerDriverConfig() { return result; } - private Map calculateOptimizedCpuDistribution(List infos) { - Map sessionsPerDriver = new HashMap<>(); + private Map calculateOptimizedCpuDistribution(List driverNames) { + Map sessionsPerDriver = new HashMap<>(); // First, allocate sessions for constrained drivers (like Safari) int remainingCores = DEFAULT_MAX_SESSIONS; - List constrainedDrivers = new ArrayList<>(); - List flexibleDrivers = new ArrayList<>(); + List flexibleDrivers = new ArrayList<>(); - for (WebDriverInfo info : infos) { - if (info.getMaximumSimultaneousSessions() == 1 - && SINGLE_SESSION_DRIVERS.contains(info.getDisplayName().toLowerCase(Locale.ENGLISH))) { - constrainedDrivers.add(info); - sessionsPerDriver.put(info, 1); + for (String driverName : driverNames) { + if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) { + // Constrained drivers get exactly 1 session + sessionsPerDriver.put(driverName, 1); remainingCores--; } else { - flexibleDrivers.add(info); + flexibleDrivers.add(driverName); } } // Then distribute remaining cores among flexible drivers - if (flexibleDrivers.size() > 0 && remainingCores > 0) { + if (!flexibleDrivers.isEmpty() && remainingCores > 0) { int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size()); int remainderCores = remainingCores % flexibleDrivers.size(); // Distribute base sessions to all flexible drivers for (int i = 0; i < flexibleDrivers.size(); i++) { - WebDriverInfo info = flexibleDrivers.get(i); + String driverName = flexibleDrivers.get(i); int sessions = sessionsPerFlexibleDriver; // Distribute remainder cores to the first 'remainderCores' drivers @@ -482,31 +478,19 @@ private Map calculateOptimizedCpuDistribution(List 0) { + } else if (!flexibleDrivers.isEmpty()) { // No remaining cores, give each flexible driver 1 session - for (WebDriverInfo info : flexibleDrivers) { - sessionsPerDriver.put(info, 1); + for (String driverName : flexibleDrivers) { + sessionsPerDriver.put(driverName, 1); } - LOG.log( - Level.FINE, - "No remaining cores available, assigning 1 session to each of {0} flexible drivers", - flexibleDrivers.size()); } return sessionsPerDriver; } - private int calculateTotalMaxSessionsFromAllDrivers(int nodeMaxSessions) { + private int calculateTotalMaxSessionsFromAllDrivers() { Map actualMaxSessions = calculateActualMaxSessionsPerDriverConfig(); return actualMaxSessions.values().stream().mapToInt(Integer::intValue).sum(); } @@ -650,8 +634,7 @@ private SessionFactory createSessionFactory(String clazz, Capabilities stereotyp private void addDriverConfigs( Function> factoryFactory, - ImmutableMultimap.Builder sessionFactories, - int maxSessions) { + ImmutableMultimap.Builder sessionFactories) { Multimap driverConfigs = HashMultimap.create(); @@ -721,10 +704,6 @@ private void addDriverConfigs( // Get actual max-sessions per driver config from centralized calculation Map actualMaxSessionsPerConfig = calculateActualMaxSessionsPerDriverConfig(); - boolean overrideMaxSessions = - config - .getBool(NODE_SECTION, "override-max-sessions") - .orElse(OVERRIDE_MAX_SESSIONS); // iterate over driver configs configList.forEach( @@ -888,7 +867,7 @@ private void addSpecificDrivers( } private Map> discoverDrivers( - int maxSessions, Function> factoryFactory) { + Function> factoryFactory) { if (!config.getBool(NODE_SECTION, "detect-drivers").orElse(DEFAULT_DETECT_DRIVERS)) { return ImmutableMap.of(); diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java index 8b4ab87037428..5ec4f29d3c4a2 100644 --- a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java @@ -614,7 +614,7 @@ void shouldNotOverrideMaxSessionsByDefault() { } // Verify node max-sessions is within CPU limit (not the overridden value) - assertThat(nodeOptions.getMaxSessions()).isLessThanOrEqualTo(maxRecommendedSessions); + assertThat(nodeOptions.getMaxSessions()).isEqualTo(maxRecommendedSessions); } @Test diff --git a/java/test/org/openqa/selenium/grid/router/StressTest.java b/java/test/org/openqa/selenium/grid/router/StressTest.java index 2631e35b6f206..d92437bd8a95a 100644 --- a/java/test/org/openqa/selenium/grid/router/StressTest.java +++ b/java/test/org/openqa/selenium/grid/router/StressTest.java @@ -66,6 +66,22 @@ class StressTest { public void setupServers() { browser = Objects.requireNonNull(Browser.detect()); + // Start app server first + appServer = + new NettyServer( + new BaseServerOptions(new MemoizedConfig(new MapConfig(Map.of()))), + req -> { + try { + Thread.sleep(2000); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return new HttpResponse().setContent(Contents.string("

Cheese

", UTF_8)); + }); + + tearDowns.add(() -> appServer.stop()); + appServer.start(); + Deployment deployment = DeploymentTypes.DISTRIBUTED.start( browser.getCapabilities(), @@ -86,21 +102,6 @@ public void setupServers() { tearDowns.add(deployment); server = deployment.getServer(); - - appServer = - new NettyServer( - new BaseServerOptions(new MemoizedConfig(new MapConfig(Map.of()))), - req -> { - try { - Thread.sleep(2000); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - return new HttpResponse().setContent(Contents.string("

Cheese

", UTF_8)); - }); - - tearDowns.add(() -> appServer.stop()); - appServer.start(); } @AfterEach diff --git a/rb/spec/unit/selenium/server_spec.rb b/rb/spec/unit/selenium/server_spec.rb index 5c3520d8d057c..b6b26b1eda1c3 100644 --- a/rb/spec/unit/selenium/server_spec.rb +++ b/rb/spec/unit/selenium/server_spec.rb @@ -23,7 +23,16 @@ module Selenium describe Server do - let(:mock_process) { instance_double(WebDriver::ChildProcess).as_null_object } + let(:mock_process) do + instance_double(WebDriver::ChildProcess).tap do |mock| + allow(mock).to receive(:start) + allow(mock).to receive(:wait) + allow(mock).to receive(:stop) + allow(mock).to receive(:detach=) + allow(mock).to receive(:io) + allow(mock).to receive(:io=) + end + end let(:mock_poller) { instance_double(WebDriver::SocketPoller, connected?: true, closed?: true) } let(:repo) { 'https://api.github.com/repos/seleniumhq/selenium/releases' } let(:port) { WebDriver::PortProber.above(4444) } @@ -49,7 +58,8 @@ module Selenium it 'uses the given jar file and port' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234') + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234', + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: 1234, background: true) @@ -58,13 +68,15 @@ module Selenium server.start expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234') + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234', + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) end it 'waits for the server process by default' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: port) @@ -75,14 +87,16 @@ module Selenium expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) expect(mock_process).to have_received(:wait) end it 'adds additional args' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, 'foo', 'bar') + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: port, background: true) @@ -94,7 +108,8 @@ module Selenium expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', - '--port', port.to_s, 'foo', 'bar') + '--port', port.to_s, '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, + 'foo', 'bar') end it 'adds additional JAVA options args' do @@ -105,6 +120,8 @@ module Selenium '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', + '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') .and_return(mock_process) @@ -197,7 +214,8 @@ module Selenium it 'raises Selenium::Server::Error if the server is not launched within the timeout' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, + '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) .and_return(mock_process) poller = instance_double(WebDriver::SocketPoller) From 8e44a4d551b0a7d504ad13e41ad264532fefe6e1 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 22 Sep 2025 12:22:42 +0700 Subject: [PATCH 4/6] Fix review comment Signed-off-by: Viet Nguyen Duc --- .../src/org/openqa/selenium/grid/node/config/NodeOptions.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index 568ff09407b66..df4a3049727b1 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -274,9 +274,7 @@ public int getMaxSessions() { return totalActualSessions; } else { // When override-max-sessions = false, return sum of actual sessions but cap at CPU cores - return totalActualSessions > 0 - ? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS) - : Math.min(maxSessions, DEFAULT_MAX_SESSIONS); + return Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS); } } From d2b9c7312c03ec39cb8d13ca2067b7e49101dd04 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 22 Sep 2025 16:32:29 +0700 Subject: [PATCH 5/6] Revert the fix Signed-off-by: Viet Nguyen Duc --- .../src/org/openqa/selenium/grid/node/config/NodeOptions.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index df4a3049727b1..568ff09407b66 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -274,7 +274,9 @@ public int getMaxSessions() { return totalActualSessions; } else { // When override-max-sessions = false, return sum of actual sessions but cap at CPU cores - return Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS); + return totalActualSessions > 0 + ? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS) + : Math.min(maxSessions, DEFAULT_MAX_SESSIONS); } } From da6fa15500ccfa26fa38739f7161ec77032930ce Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Thu, 25 Sep 2025 08:32:47 +0700 Subject: [PATCH 6/6] Update logic for detect-drivers true Signed-off-by: Viet Nguyen Duc --- .../grid/node/config/NodeOptions.java | 105 +++++++++++------- .../grid/node/config/NodeOptionsTest.java | 21 ++-- py/selenium/webdriver/remote/server.py | 5 - rb/lib/selenium/server.rb | 15 --- rb/spec/unit/selenium/server_spec.rb | 37 ++---- 5 files changed, 83 insertions(+), 100 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java index 568ff09407b66..c776777ba1ac6 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java @@ -273,10 +273,16 @@ public int getMaxSessions() { if (overrideMaxSessions) { return totalActualSessions; } else { - // When override-max-sessions = false, return sum of actual sessions but cap at CPU cores - return totalActualSessions > 0 - ? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS) - : Math.min(maxSessions, DEFAULT_MAX_SESSIONS); + // When explicit max-sessions is provided and within CPU cores, use it; otherwise cap at CPU + // cores + boolean hasExplicitMaxSessions = config.get(NODE_SECTION, "max-sessions").isPresent(); + if (hasExplicitMaxSessions && maxSessions <= DEFAULT_MAX_SESSIONS) { + return maxSessions; + } else { + return totalActualSessions > 0 + ? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS) + : Math.min(maxSessions, DEFAULT_MAX_SESSIONS); + } } } @@ -368,7 +374,9 @@ private Map calculateActualMaxSessionsPerDriverConfig() { configList.stream() .map(config -> config.get("display-name")) .collect(Collectors.toList()); - Map sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames); + boolean hasExplicitMaxSessions = nodeMaxSessions > DEFAULT_MAX_SESSIONS; + Map sessionsPerDriver = + calculateOptimizedCpuDistribution(driverNames, false, hasExplicitMaxSessions); for (Map configMap : configList) { String displayName = configMap.get("display-name"); @@ -422,10 +430,12 @@ private Map calculateActualMaxSessionsPerDriverConfig() { detectedDrivers.stream() .map(WebDriverInfo::getDisplayName) .collect(Collectors.toList()); - Map sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames); + boolean hasExplicitMaxSessions = false; + Map sessionsPerDriver = + calculateOptimizedCpuDistribution(driverNames, true, hasExplicitMaxSessions); // Check if node max-sessions is explicitly set and within allowed range - if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) { + if (nodeMaxSessions < DEFAULT_MAX_SESSIONS) { for (WebDriverInfo info : detectedDrivers) { int calculatedSessions = sessionsPerDriver.get(info.getDisplayName()); if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) { @@ -435,9 +445,7 @@ private Map calculateActualMaxSessionsPerDriverConfig() { } } } else { - for (Map.Entry entry : sessionsPerDriver.entrySet()) { - result.put(entry.getKey(), entry.getValue()); - } + result.putAll(sessionsPerDriver); } } } @@ -446,44 +454,59 @@ private Map calculateActualMaxSessionsPerDriverConfig() { return result; } - private Map calculateOptimizedCpuDistribution(List driverNames) { + private Map calculateOptimizedCpuDistribution( + List driverNames, boolean detectDrivers, boolean hasExplicitMaxSessions) { Map sessionsPerDriver = new HashMap<>(); - // First, allocate sessions for constrained drivers (like Safari) - int remainingCores = DEFAULT_MAX_SESSIONS; - List flexibleDrivers = new ArrayList<>(); - - for (String driverName : driverNames) { - if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) { - // Constrained drivers get exactly 1 session - sessionsPerDriver.put(driverName, 1); - remainingCores--; - } else { - flexibleDrivers.add(driverName); + // When detect-drivers is true and no explicit max-sessions is provided, + // each driver should get the full number of processors (except single-session drivers) + if (detectDrivers && !hasExplicitMaxSessions) { + for (String driverName : driverNames) { + if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) { + // Constrained drivers (like Safari) get exactly 1 session + sessionsPerDriver.put(driverName, 1); + } else { + // Flexible drivers get the full number of available processors + sessionsPerDriver.put(driverName, DEFAULT_MAX_SESSIONS); + } + } + } else { + // Original logic: distribute cores among drivers + int remainingCores = DEFAULT_MAX_SESSIONS; + List flexibleDrivers = new ArrayList<>(); + + for (String driverName : driverNames) { + if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) { + // Constrained drivers get exactly 1 session + sessionsPerDriver.put(driverName, 1); + remainingCores--; + } else { + flexibleDrivers.add(driverName); + } } - } - // Then distribute remaining cores among flexible drivers - if (!flexibleDrivers.isEmpty() && remainingCores > 0) { - int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size()); - int remainderCores = remainingCores % flexibleDrivers.size(); + // Then distribute remaining cores among flexible drivers + if (!flexibleDrivers.isEmpty() && remainingCores > 0) { + int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size()); + int remainderCores = remainingCores % flexibleDrivers.size(); - // Distribute base sessions to all flexible drivers - for (int i = 0; i < flexibleDrivers.size(); i++) { - String driverName = flexibleDrivers.get(i); - int sessions = sessionsPerFlexibleDriver; + // Distribute base sessions to all flexible drivers + for (int i = 0; i < flexibleDrivers.size(); i++) { + String driverName = flexibleDrivers.get(i); + int sessions = sessionsPerFlexibleDriver; - // Distribute remainder cores to the first 'remainderCores' drivers - if (i < remainderCores) { - sessions++; - } + // Distribute remainder cores to the first 'remainderCores' drivers + if (i < remainderCores) { + sessions++; + } - sessionsPerDriver.put(driverName, sessions); - } - } else if (!flexibleDrivers.isEmpty()) { - // No remaining cores, give each flexible driver 1 session - for (String driverName : flexibleDrivers) { - sessionsPerDriver.put(driverName, 1); + sessionsPerDriver.put(driverName, sessions); + } + } else if (!flexibleDrivers.isEmpty()) { + // No remaining cores, give each flexible driver 1 session + for (String driverName : flexibleDrivers) { + sessionsPerDriver.put(driverName, 1); + } } } diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java index 5ec4f29d3c4a2..babd6692aac39 100644 --- a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java @@ -600,21 +600,22 @@ void shouldNotOverrideMaxSessionsByDefault() { int overriddenMaxSessions = maxRecommendedSessions + 10; Config config = new MapConfig(singletonMap("node", ImmutableMap.of("max-sessions", overriddenMaxSessions))); - - NodeOptions nodeOptions = new NodeOptions(config); List reported = new ArrayList<>(); try { - nodeOptions.getSessionFactories( - caps -> { - reported.add(caps); - return Collections.singleton(HelperFactory.create(config, caps)); - }); + new NodeOptions(config) + .getSessionFactories( + caps -> { + reported.add(caps); + return Collections.singleton(HelperFactory.create(config, caps)); + }); } catch (ConfigException e) { // Fall through } - - // Verify node max-sessions is within CPU limit (not the overridden value) - assertThat(nodeOptions.getMaxSessions()).isEqualTo(maxRecommendedSessions); + long chromeSlots = + reported.stream() + .filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName())) + .count(); + assertThat(chromeSlots).isEqualTo(maxRecommendedSessions); } @Test diff --git a/py/selenium/webdriver/remote/server.py b/py/selenium/webdriver/remote/server.py index d461d100da4a2..09e9e647331fe 100644 --- a/py/selenium/webdriver/remote/server.py +++ b/py/selenium/webdriver/remote/server.py @@ -16,7 +16,6 @@ # under the License. import collections -import multiprocessing import os import re import shutil @@ -171,10 +170,6 @@ def start(self): "true", "--enable-managed-downloads", "true", - "--override-max-sessions", - "true", - "--max-sessions", - str(multiprocessing.cpu_count()), ] if self.host is not None: command.extend(["--host", self.host]) diff --git a/rb/lib/selenium/server.rb b/rb/lib/selenium/server.rb index 342274c3e227b..ef2b05e99f199 100644 --- a/rb/lib/selenium/server.rb +++ b/rb/lib/selenium/server.rb @@ -21,7 +21,6 @@ require 'selenium/webdriver/common/port_prober' require 'selenium/webdriver/common/socket_poller' require 'net/http' -require 'etc' module Selenium # @@ -178,8 +177,6 @@ def download_server(uri, destination) # @option opts [true,false] :background Run the server in the background (default: false) # @option opts [true,false,String] :log Either a path to a log file, # or true to pass server log to stdout. - # @option opts [true,false] :override_max_sessions Override the maximum number of sessions - # @option opts [Integer] :max_sessions Maximum number of concurrent sessions # @raise [Errno::ENOENT] if the jar file does not exist # @@ -201,18 +198,6 @@ def initialize(jar, opts = {}) @additional_args << opts[:log_level].to_s end - override_max_sessions = opts.fetch(:override_max_sessions, true) - if override_max_sessions - @additional_args << '--override-max-sessions' - @additional_args << override_max_sessions.to_s - end - - max_sessions = opts.fetch(:max_sessions, Etc.nprocessors) - if max_sessions - @additional_args << '--max-sessions' - @additional_args << max_sessions.to_s - end - @log_file = nil end diff --git a/rb/spec/unit/selenium/server_spec.rb b/rb/spec/unit/selenium/server_spec.rb index b6b26b1eda1c3..eedd755dd4131 100644 --- a/rb/spec/unit/selenium/server_spec.rb +++ b/rb/spec/unit/selenium/server_spec.rb @@ -18,21 +18,11 @@ # under the License. require File.expand_path('webdriver/spec_helper', __dir__) -require 'etc' require 'selenium/server' module Selenium describe Server do - let(:mock_process) do - instance_double(WebDriver::ChildProcess).tap do |mock| - allow(mock).to receive(:start) - allow(mock).to receive(:wait) - allow(mock).to receive(:stop) - allow(mock).to receive(:detach=) - allow(mock).to receive(:io) - allow(mock).to receive(:io=) - end - end + let(:mock_process) { instance_double(WebDriver::ChildProcess).as_null_object } let(:mock_poller) { instance_double(WebDriver::SocketPoller, connected?: true, closed?: true) } let(:repo) { 'https://api.github.com/repos/seleniumhq/selenium/releases' } let(:port) { WebDriver::PortProber.above(4444) } @@ -58,8 +48,7 @@ module Selenium it 'uses the given jar file and port' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234', - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234') .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: 1234, background: true) @@ -68,15 +57,13 @@ module Selenium server.start expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234', - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234') end it 'waits for the server process by default' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: port) @@ -87,16 +74,14 @@ module Selenium expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) expect(mock_process).to have_received(:wait) end it 'adds additional args' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, 'foo', 'bar') .and_return(mock_process) server = described_class.new('selenium_server_deploy.jar', port: port, background: true) @@ -108,8 +93,7 @@ module Selenium expect(File).to have_received(:exist?).with('selenium_server_deploy.jar') expect(WebDriver::ChildProcess).to have_received(:build) .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', - '--port', port.to_s, '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, - 'foo', 'bar') + '--port', port.to_s, 'foo', 'bar') end it 'adds additional JAVA options args' do @@ -120,8 +104,6 @@ module Selenium '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', - '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') .and_return(mock_process) @@ -140,8 +122,6 @@ module Selenium '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', - '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar') end @@ -214,8 +194,7 @@ module Selenium it 'raises Selenium::Server::Error if the server is not launched within the timeout' do allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true) allow(WebDriver::ChildProcess).to receive(:build) - .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, - '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s) + .with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s) .and_return(mock_process) poller = instance_double(WebDriver::SocketPoller)