[grid] Enhance max-sessions in case specify driver configuration in Node Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
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 5afc047..d2a290a 100644 --- a/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java +++ b/java/src/org/openqa/selenium/grid/node/config/NodeOptions.java
@@ -454,6 +454,10 @@ private void addDriverConfigs( throw new ConfigException("No driver configs were found!"); } + // Handle session distribution for detect-drivers = false scenario + Map<String, Integer> sessionDistribution = + calculateDriverConfigSessionDistribution(configList, maxSessions); + List<DriverService.Builder<?, ?>> builderList = new ArrayList<>(); ServiceLoader.load(DriverService.Builder.class).forEach(builderList::add); @@ -506,9 +510,8 @@ private void addDriverConfigs( new ConfigException( "Unable to find matching driver for %s", stereotype)); - int driverMaxSessions = - Integer.parseInt( - thisConfig.getOrDefault("max-sessions", String.valueOf(maxSessions))); + // Use calculated session distribution + int driverMaxSessions = sessionDistribution.get(configName); Require.positive("Driver max sessions", driverMaxSessions); WebDriverInfo driverInfoConfig = @@ -521,8 +524,7 @@ private void addDriverConfigs( builder -> { ImmutableCapabilities immutable = new ImmutableCapabilities(stereotype); - int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions); - for (int i = 0; i < maxDriverSessions; i++) { + for (int i = 0; i < driverMaxSessions; i++) { driverConfigs.putAll( driverInfoConfig, factoryFactory.apply(immutable)); } @@ -792,6 +794,75 @@ private void report(Map.Entry<WebDriverInfo, Collection<SessionFactory>> entry) entry.getValue().size())); } + private Map<String, Integer> calculateDriverConfigSessionDistribution( + List<Map<String, String>> configList, int nodeMaxSessions) { + Map<String, Integer> sessionDistribution = new HashMap<>(); + boolean overrideMaxSessions = + config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS); + + // S2.1: No max-sessions given, distribute CPU cores among driver configurations + if (!config.getInt(NODE_SECTION, "max-sessions").isPresent()) { + int availableCores = DEFAULT_MAX_SESSIONS; + int numDrivers = configList.size(); + + // Distribute cores evenly, with remainder going to first drivers + int baseSessions = availableCores / numDrivers; + int remainder = availableCores % numDrivers; + + for (int i = 0; i < configList.size(); i++) { + Map<String, String> driverConfig = configList.get(i); + String displayName = driverConfig.get("display-name"); + + // Check if driver has specific max-sessions (S2.3) + int driverSessions; + if (driverConfig.containsKey("max-sessions")) { + int specificMaxSessions = Integer.parseInt(driverConfig.get("max-sessions")); + if (overrideMaxSessions) { + driverSessions = specificMaxSessions; + } else { + // Respect CPU core distribution even with specific max-sessions + driverSessions = Math.min(specificMaxSessions, baseSessions + (i < remainder ? 1 : 0)); + } + } else { + driverSessions = baseSessions + (i < remainder ? 1 : 0); + } + + sessionDistribution.put(displayName, driverSessions); + } + } else { + // S2.2: Given max-sessions under [node] section + int configuredMaxSessions = nodeMaxSessions; + + for (Map<String, String> driverConfig : configList) { + String displayName = driverConfig.get("display-name"); + + // Check if driver has specific max-sessions (S2.3) + int driverSessions; + if (driverConfig.containsKey("max-sessions")) { + int specificMaxSessions = Integer.parseInt(driverConfig.get("max-sessions")); + if (overrideMaxSessions) { + // S2.4: Unlimited configure max-sessions with override-max-sessions true + driverSessions = specificMaxSessions; + } else { + // S2.3: Specific driver max-sessions takes precedence but controlled by CPU cores + driverSessions = Math.min(specificMaxSessions, configuredMaxSessions); + } + } else { + // Use node max-sessions, but respect CPU limits if override is false + if (overrideMaxSessions) { + driverSessions = configuredMaxSessions; + } else { + driverSessions = Math.min(configuredMaxSessions, DEFAULT_MAX_SESSIONS); + } + } + + sessionDistribution.put(displayName, driverSessions); + } + } + + return sessionDistribution; + } + private String unquote(String input) { int len = input.length(); if ((input.charAt(0) == '"') && (input.charAt(len - 1) == '"')) {
diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsDetectDriversFalseTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsDetectDriversFalseTest.java new file mode 100644 index 0000000..7afa8d4 --- /dev/null +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsDetectDriversFalseTest.java
@@ -0,0 +1,105 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + +package org.openqa.selenium.grid.node.config; + +import static java.util.Collections.singleton; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.Capabilities; +import org.openqa.selenium.grid.config.Config; +import org.openqa.selenium.grid.config.TomlConfig; +import org.openqa.selenium.grid.node.SessionFactory; + +class NodeOptionsDetectDriversFalseTest { + + @Test + void testS24_UnlimitedMaxSessionsWithOverride() { + String[] rawConfig = + new String[] { + "[node]", + "max-sessions = 4", + "override-max-sessions = true", + "detect-drivers = false", + "", + "[[node.driver-configuration]]", + "display-name = \"chrome\"", + "stereotype = '{\"browserName\":\"chrome\"}'", + "", + "[[node.driver-configuration]]", + "display-name = \"MicrosoftEdge\"", + "stereotype = '{\"browserName\":\"MicrosoftEdge\"}'", + "max-sessions = 3", + "", + "[[node.driver-configuration]]", + "display-name = \"firefox\"", + "stereotype = '{\"browserName\":\"firefox\"}'", + "max-sessions = 5" + }; + + Config config = new TomlConfig(new StringReader(String.join("\n", rawConfig))); + List<Capabilities> reported = new ArrayList<>(); + + new NodeOptions(config) + .getSessionFactories( + caps -> { + reported.add(caps); + return singleton(HelperFactory.create(config, caps)); + }); + + long chromeCount = + reported.stream().filter(caps -> "chrome".equals(caps.getBrowserName())).count(); + long edgeCount = + reported.stream().filter(caps -> "MicrosoftEdge".equals(caps.getBrowserName())).count(); + long firefoxCount = + reported.stream().filter(caps -> "firefox".equals(caps.getBrowserName())).count(); + + assertThat(chromeCount).isEqualTo(4); + assertThat(edgeCount).isEqualTo(3); + assertThat(firefoxCount).isEqualTo(5); + assertThat(reported.size()).isEqualTo(12); + } + + public static class HelperFactory { + public static SessionFactory create(Config config, Capabilities caps) { + return new SessionFactory() { + @Override + public Capabilities getStereotype() { + return caps; + } + + @Override + public org.openqa.selenium.internal.Either< + org.openqa.selenium.WebDriverException, org.openqa.selenium.grid.node.ActiveSession> + apply(org.openqa.selenium.grid.data.CreateSessionRequest createSessionRequest) { + return org.openqa.selenium.internal.Either.left( + new org.openqa.selenium.SessionNotCreatedException("HelperFactory for testing")); + } + + @Override + public boolean test(Capabilities capabilities) { + return true; + } + }; + } + } +}