Skip to content

Commit 80526af

Browse files
VietND96VietND96
committed
[grid] Enhance max-sessions in case specify driver configuration in Node
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent 1347c78 commit 80526af

File tree

2 files changed

+181
-5
lines changed

2 files changed

+181
-5
lines changed

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

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,10 @@ private void addDriverConfigs(
454454
throw new ConfigException("No driver configs were found!");
455455
}
456456

457+
// Handle session distribution for detect-drivers = false scenario
458+
Map<String, Integer> sessionDistribution =
459+
calculateDriverConfigSessionDistribution(configList, maxSessions);
460+
457461
List<DriverService.Builder<?, ?>> builderList = new ArrayList<>();
458462
ServiceLoader.load(DriverService.Builder.class).forEach(builderList::add);
459463

@@ -506,9 +510,8 @@ private void addDriverConfigs(
506510
new ConfigException(
507511
"Unable to find matching driver for %s", stereotype));
508512

509-
int driverMaxSessions =
510-
Integer.parseInt(
511-
thisConfig.getOrDefault("max-sessions", String.valueOf(maxSessions)));
513+
// Use calculated session distribution
514+
int driverMaxSessions = sessionDistribution.get(configName);
512515
Require.positive("Driver max sessions", driverMaxSessions);
513516

514517
WebDriverInfo driverInfoConfig =
@@ -521,8 +524,7 @@ private void addDriverConfigs(
521524
builder -> {
522525
ImmutableCapabilities immutable =
523526
new ImmutableCapabilities(stereotype);
524-
int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions);
525-
for (int i = 0; i < maxDriverSessions; i++) {
527+
for (int i = 0; i < driverMaxSessions; i++) {
526528
driverConfigs.putAll(
527529
driverInfoConfig, factoryFactory.apply(immutable));
528530
}
@@ -792,6 +794,75 @@ private void report(Map.Entry<WebDriverInfo, Collection<SessionFactory>> entry)
792794
entry.getValue().size()));
793795
}
794796

797+
private Map<String, Integer> calculateDriverConfigSessionDistribution(
798+
List<Map<String, String>> configList, int nodeMaxSessions) {
799+
Map<String, Integer> sessionDistribution = new HashMap<>();
800+
boolean overrideMaxSessions =
801+
config.getBool(NODE_SECTION, "override-max-sessions").orElse(OVERRIDE_MAX_SESSIONS);
802+
803+
// S2.1: No max-sessions given, distribute CPU cores among driver configurations
804+
if (!config.getInt(NODE_SECTION, "max-sessions").isPresent()) {
805+
int availableCores = DEFAULT_MAX_SESSIONS;
806+
int numDrivers = configList.size();
807+
808+
// Distribute cores evenly, with remainder going to first drivers
809+
int baseSessions = availableCores / numDrivers;
810+
int remainder = availableCores % numDrivers;
811+
812+
for (int i = 0; i < configList.size(); i++) {
813+
Map<String, String> driverConfig = configList.get(i);
814+
String displayName = driverConfig.get("display-name");
815+
816+
// Check if driver has specific max-sessions (S2.3)
817+
int driverSessions;
818+
if (driverConfig.containsKey("max-sessions")) {
819+
int specificMaxSessions = Integer.parseInt(driverConfig.get("max-sessions"));
820+
if (overrideMaxSessions) {
821+
driverSessions = specificMaxSessions;
822+
} else {
823+
// Respect CPU core distribution even with specific max-sessions
824+
driverSessions = Math.min(specificMaxSessions, baseSessions + (i < remainder ? 1 : 0));
825+
}
826+
} else {
827+
driverSessions = baseSessions + (i < remainder ? 1 : 0);
828+
}
829+
830+
sessionDistribution.put(displayName, driverSessions);
831+
}
832+
} else {
833+
// S2.2: Given max-sessions under [node] section
834+
int configuredMaxSessions = nodeMaxSessions;
835+
836+
for (Map<String, String> driverConfig : configList) {
837+
String displayName = driverConfig.get("display-name");
838+
839+
// Check if driver has specific max-sessions (S2.3)
840+
int driverSessions;
841+
if (driverConfig.containsKey("max-sessions")) {
842+
int specificMaxSessions = Integer.parseInt(driverConfig.get("max-sessions"));
843+
if (overrideMaxSessions) {
844+
// S2.4: Unlimited configure max-sessions with override-max-sessions true
845+
driverSessions = specificMaxSessions;
846+
} else {
847+
// S2.3: Specific driver max-sessions takes precedence but controlled by CPU cores
848+
driverSessions = Math.min(specificMaxSessions, configuredMaxSessions);
849+
}
850+
} else {
851+
// Use node max-sessions, but respect CPU limits if override is false
852+
if (overrideMaxSessions) {
853+
driverSessions = configuredMaxSessions;
854+
} else {
855+
driverSessions = Math.min(configuredMaxSessions, DEFAULT_MAX_SESSIONS);
856+
}
857+
}
858+
859+
sessionDistribution.put(displayName, driverSessions);
860+
}
861+
}
862+
863+
return sessionDistribution;
864+
}
865+
795866
private String unquote(String input) {
796867
int len = input.length();
797868
if ((input.charAt(0) == '"') && (input.charAt(len - 1) == '"')) {

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
19+
package org.openqa.selenium.grid.node.config;
20+
21+
import static java.util.Collections.singleton;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
24+
import java.io.StringReader;
25+
import java.util.ArrayList;
26+
import java.util.List;
27+
import org.junit.jupiter.api.Test;
28+
import org.openqa.selenium.Capabilities;
29+
import org.openqa.selenium.grid.config.Config;
30+
import org.openqa.selenium.grid.config.TomlConfig;
31+
import org.openqa.selenium.grid.node.SessionFactory;
32+
33+
class NodeOptionsDetectDriversFalseTest {
34+
35+
@Test
36+
void testS24_UnlimitedMaxSessionsWithOverride() {
37+
String[] rawConfig =
38+
new String[] {
39+
"[node]",
40+
"max-sessions = 4",
41+
"override-max-sessions = true",
42+
"detect-drivers = false",
43+
"",
44+
"[[node.driver-configuration]]",
45+
"display-name = \"chrome\"",
46+
"stereotype = '{\"browserName\":\"chrome\"}'",
47+
"",
48+
"[[node.driver-configuration]]",
49+
"display-name = \"MicrosoftEdge\"",
50+
"stereotype = '{\"browserName\":\"MicrosoftEdge\"}'",
51+
"max-sessions = 3",
52+
"",
53+
"[[node.driver-configuration]]",
54+
"display-name = \"firefox\"",
55+
"stereotype = '{\"browserName\":\"firefox\"}'",
56+
"max-sessions = 5"
57+
};
58+
59+
Config config = new TomlConfig(new StringReader(String.join("\n", rawConfig)));
60+
List<Capabilities> reported = new ArrayList<>();
61+
62+
new NodeOptions(config)
63+
.getSessionFactories(
64+
caps -> {
65+
reported.add(caps);
66+
return singleton(HelperFactory.create(config, caps));
67+
});
68+
69+
long chromeCount =
70+
reported.stream().filter(caps -> "chrome".equals(caps.getBrowserName())).count();
71+
long edgeCount =
72+
reported.stream().filter(caps -> "MicrosoftEdge".equals(caps.getBrowserName())).count();
73+
long firefoxCount =
74+
reported.stream().filter(caps -> "firefox".equals(caps.getBrowserName())).count();
75+
76+
assertThat(chromeCount).isEqualTo(4);
77+
assertThat(edgeCount).isEqualTo(3);
78+
assertThat(firefoxCount).isEqualTo(5);
79+
assertThat(reported.size()).isEqualTo(12);
80+
}
81+
82+
public static class HelperFactory {
83+
public static SessionFactory create(Config config, Capabilities caps) {
84+
return new SessionFactory() {
85+
@Override
86+
public Capabilities getStereotype() {
87+
return caps;
88+
}
89+
90+
@Override
91+
public org.openqa.selenium.internal.Either<
92+
org.openqa.selenium.WebDriverException, org.openqa.selenium.grid.node.ActiveSession>
93+
apply(org.openqa.selenium.grid.data.CreateSessionRequest createSessionRequest) {
94+
return org.openqa.selenium.internal.Either.left(
95+
new org.openqa.selenium.SessionNotCreatedException("HelperFactory for testing"));
96+
}
97+
98+
@Override
99+
public boolean test(Capabilities capabilities) {
100+
return true;
101+
}
102+
};
103+
}
104+
}
105+
}

0 commit comments

Comments
 (0)