Skip to content

Commit da6fa15

Browse files
VietND96VietND96
committed
Update logic for detect-drivers true
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent d2b9c73 commit da6fa15

File tree

5 files changed

+83
-100
lines changed

5 files changed

+83
-100
lines changed

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

Lines changed: 64 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -273,10 +273,16 @@ public int getMaxSessions() {
273273
if (overrideMaxSessions) {
274274
return totalActualSessions;
275275
} else {
276-
// When override-max-sessions = false, return sum of actual sessions but cap at CPU cores
277-
return totalActualSessions > 0
278-
? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS)
279-
: Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
276+
// When explicit max-sessions is provided and within CPU cores, use it; otherwise cap at CPU
277+
// cores
278+
boolean hasExplicitMaxSessions = config.get(NODE_SECTION, "max-sessions").isPresent();
279+
if (hasExplicitMaxSessions && maxSessions <= DEFAULT_MAX_SESSIONS) {
280+
return maxSessions;
281+
} else {
282+
return totalActualSessions > 0
283+
? Math.min(totalActualSessions, DEFAULT_MAX_SESSIONS)
284+
: Math.min(maxSessions, DEFAULT_MAX_SESSIONS);
285+
}
280286
}
281287
}
282288

@@ -368,7 +374,9 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
368374
configList.stream()
369375
.map(config -> config.get("display-name"))
370376
.collect(Collectors.toList());
371-
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
377+
boolean hasExplicitMaxSessions = nodeMaxSessions > DEFAULT_MAX_SESSIONS;
378+
Map<String, Integer> sessionsPerDriver =
379+
calculateOptimizedCpuDistribution(driverNames, false, hasExplicitMaxSessions);
372380

373381
for (Map<String, String> configMap : configList) {
374382
String displayName = configMap.get("display-name");
@@ -422,10 +430,12 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
422430
detectedDrivers.stream()
423431
.map(WebDriverInfo::getDisplayName)
424432
.collect(Collectors.toList());
425-
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
433+
boolean hasExplicitMaxSessions = false;
434+
Map<String, Integer> sessionsPerDriver =
435+
calculateOptimizedCpuDistribution(driverNames, true, hasExplicitMaxSessions);
426436

427437
// Check if node max-sessions is explicitly set and within allowed range
428-
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
438+
if (nodeMaxSessions < DEFAULT_MAX_SESSIONS) {
429439
for (WebDriverInfo info : detectedDrivers) {
430440
int calculatedSessions = sessionsPerDriver.get(info.getDisplayName());
431441
if (nodeMaxSessions >= 1 && nodeMaxSessions <= calculatedSessions) {
@@ -435,9 +445,7 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
435445
}
436446
}
437447
} else {
438-
for (Map.Entry<String, Integer> entry : sessionsPerDriver.entrySet()) {
439-
result.put(entry.getKey(), entry.getValue());
440-
}
448+
result.putAll(sessionsPerDriver);
441449
}
442450
}
443451
}
@@ -446,44 +454,59 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
446454
return result;
447455
}
448456

449-
private Map<String, Integer> calculateOptimizedCpuDistribution(List<String> driverNames) {
457+
private Map<String, Integer> calculateOptimizedCpuDistribution(
458+
List<String> driverNames, boolean detectDrivers, boolean hasExplicitMaxSessions) {
450459
Map<String, Integer> sessionsPerDriver = new HashMap<>();
451460

452-
// First, allocate sessions for constrained drivers (like Safari)
453-
int remainingCores = DEFAULT_MAX_SESSIONS;
454-
List<String> flexibleDrivers = new ArrayList<>();
455-
456-
for (String driverName : driverNames) {
457-
if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) {
458-
// Constrained drivers get exactly 1 session
459-
sessionsPerDriver.put(driverName, 1);
460-
remainingCores--;
461-
} else {
462-
flexibleDrivers.add(driverName);
461+
// When detect-drivers is true and no explicit max-sessions is provided,
462+
// each driver should get the full number of processors (except single-session drivers)
463+
if (detectDrivers && !hasExplicitMaxSessions) {
464+
for (String driverName : driverNames) {
465+
if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) {
466+
// Constrained drivers (like Safari) get exactly 1 session
467+
sessionsPerDriver.put(driverName, 1);
468+
} else {
469+
// Flexible drivers get the full number of available processors
470+
sessionsPerDriver.put(driverName, DEFAULT_MAX_SESSIONS);
471+
}
472+
}
473+
} else {
474+
// Original logic: distribute cores among drivers
475+
int remainingCores = DEFAULT_MAX_SESSIONS;
476+
List<String> flexibleDrivers = new ArrayList<>();
477+
478+
for (String driverName : driverNames) {
479+
if (SINGLE_SESSION_DRIVERS.contains(driverName.toLowerCase(Locale.ENGLISH))) {
480+
// Constrained drivers get exactly 1 session
481+
sessionsPerDriver.put(driverName, 1);
482+
remainingCores--;
483+
} else {
484+
flexibleDrivers.add(driverName);
485+
}
463486
}
464-
}
465487

466-
// Then distribute remaining cores among flexible drivers
467-
if (!flexibleDrivers.isEmpty() && remainingCores > 0) {
468-
int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size());
469-
int remainderCores = remainingCores % flexibleDrivers.size();
488+
// Then distribute remaining cores among flexible drivers
489+
if (!flexibleDrivers.isEmpty() && remainingCores > 0) {
490+
int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size());
491+
int remainderCores = remainingCores % flexibleDrivers.size();
470492

471-
// Distribute base sessions to all flexible drivers
472-
for (int i = 0; i < flexibleDrivers.size(); i++) {
473-
String driverName = flexibleDrivers.get(i);
474-
int sessions = sessionsPerFlexibleDriver;
493+
// Distribute base sessions to all flexible drivers
494+
for (int i = 0; i < flexibleDrivers.size(); i++) {
495+
String driverName = flexibleDrivers.get(i);
496+
int sessions = sessionsPerFlexibleDriver;
475497

476-
// Distribute remainder cores to the first 'remainderCores' drivers
477-
if (i < remainderCores) {
478-
sessions++;
479-
}
498+
// Distribute remainder cores to the first 'remainderCores' drivers
499+
if (i < remainderCores) {
500+
sessions++;
501+
}
480502

481-
sessionsPerDriver.put(driverName, sessions);
482-
}
483-
} else if (!flexibleDrivers.isEmpty()) {
484-
// No remaining cores, give each flexible driver 1 session
485-
for (String driverName : flexibleDrivers) {
486-
sessionsPerDriver.put(driverName, 1);
503+
sessionsPerDriver.put(driverName, sessions);
504+
}
505+
} else if (!flexibleDrivers.isEmpty()) {
506+
// No remaining cores, give each flexible driver 1 session
507+
for (String driverName : flexibleDrivers) {
508+
sessionsPerDriver.put(driverName, 1);
509+
}
487510
}
488511
}
489512

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -600,21 +600,22 @@ 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);
605603
List<Capabilities> reported = new ArrayList<>();
606604
try {
607-
nodeOptions.getSessionFactories(
608-
caps -> {
609-
reported.add(caps);
610-
return Collections.singleton(HelperFactory.create(config, caps));
611-
});
605+
new NodeOptions(config)
606+
.getSessionFactories(
607+
caps -> {
608+
reported.add(caps);
609+
return Collections.singleton(HelperFactory.create(config, caps));
610+
});
612611
} catch (ConfigException e) {
613612
// Fall through
614613
}
615-
616-
// Verify node max-sessions is within CPU limit (not the overridden value)
617-
assertThat(nodeOptions.getMaxSessions()).isEqualTo(maxRecommendedSessions);
614+
long chromeSlots =
615+
reported.stream()
616+
.filter(capabilities -> "chrome".equalsIgnoreCase(capabilities.getBrowserName()))
617+
.count();
618+
assertThat(chromeSlots).isEqualTo(maxRecommendedSessions);
618619
}
619620

620621
@Test

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

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

1818
import collections
19-
import multiprocessing
2019
import os
2120
import re
2221
import shutil
@@ -171,10 +170,6 @@ def start(self):
171170
"true",
172171
"--enable-managed-downloads",
173172
"true",
174-
"--override-max-sessions",
175-
"true",
176-
"--max-sessions",
177-
str(multiprocessing.cpu_count()),
178173
]
179174
if self.host is not None:
180175
command.extend(["--host", self.host])

‎rb/lib/selenium/server.rb‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
require 'selenium/webdriver/common/port_prober'
2222
require 'selenium/webdriver/common/socket_poller'
2323
require 'net/http'
24-
require 'etc'
2524

2625
module Selenium
2726
#
@@ -178,8 +177,6 @@ def download_server(uri, destination)
178177
# @option opts [true,false] :background Run the server in the background (default: false)
179178
# @option opts [true,false,String] :log Either a path to a log file,
180179
# or true to pass server log to stdout.
181-
# @option opts [true,false] :override_max_sessions Override the maximum number of sessions
182-
# @option opts [Integer] :max_sessions Maximum number of concurrent sessions
183180
# @raise [Errno::ENOENT] if the jar file does not exist
184181
#
185182

@@ -201,18 +198,6 @@ def initialize(jar, opts = {})
201198
@additional_args << opts[:log_level].to_s
202199
end
203200

204-
override_max_sessions = opts.fetch(:override_max_sessions, true)
205-
if override_max_sessions
206-
@additional_args << '--override-max-sessions'
207-
@additional_args << override_max_sessions.to_s
208-
end
209-
210-
max_sessions = opts.fetch(:max_sessions, Etc.nprocessors)
211-
if max_sessions
212-
@additional_args << '--max-sessions'
213-
@additional_args << max_sessions.to_s
214-
end
215-
216201
@log_file = nil
217202
end
218203

‎rb/spec/unit/selenium/server_spec.rb‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,11 @@
1818
# under the License.
1919

2020
require File.expand_path('webdriver/spec_helper', __dir__)
21-
require 'etc'
2221
require 'selenium/server'
2322

2423
module Selenium
2524
describe Server do
26-
let(:mock_process) do
27-
instance_double(WebDriver::ChildProcess).tap do |mock|
28-
allow(mock).to receive(:start)
29-
allow(mock).to receive(:wait)
30-
allow(mock).to receive(:stop)
31-
allow(mock).to receive(:detach=)
32-
allow(mock).to receive(:io)
33-
allow(mock).to receive(:io=)
34-
end
35-
end
25+
let(:mock_process) { instance_double(WebDriver::ChildProcess).as_null_object }
3626
let(:mock_poller) { instance_double(WebDriver::SocketPoller, connected?: true, closed?: true) }
3727
let(:repo) { 'https://api.github.com/repos/seleniumhq/selenium/releases' }
3828
let(:port) { WebDriver::PortProber.above(4444) }
@@ -58,8 +48,7 @@ module Selenium
5848
it 'uses the given jar file and port' do
5949
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
6050
allow(WebDriver::ChildProcess).to receive(:build)
61-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234',
62-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
51+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234')
6352
.and_return(mock_process)
6453

6554
server = described_class.new('selenium_server_deploy.jar', port: 1234, background: true)
@@ -68,15 +57,13 @@ module Selenium
6857
server.start
6958
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
7059
expect(WebDriver::ChildProcess).to have_received(:build)
71-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234',
72-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
60+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', '1234')
7361
end
7462

7563
it 'waits for the server process by default' do
7664
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
7765
allow(WebDriver::ChildProcess).to receive(:build)
78-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
79-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
66+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
8067
.and_return(mock_process)
8168

8269
server = described_class.new('selenium_server_deploy.jar', port: port)
@@ -87,16 +74,14 @@ module Selenium
8774

8875
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
8976
expect(WebDriver::ChildProcess).to have_received(:build)
90-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
91-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
77+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
9278
expect(mock_process).to have_received(:wait)
9379
end
9480

9581
it 'adds additional args' do
9682
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
9783
allow(WebDriver::ChildProcess).to receive(:build)
98-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
99-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s, 'foo', 'bar')
84+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s, 'foo', 'bar')
10085
.and_return(mock_process)
10186

10287
server = described_class.new('selenium_server_deploy.jar', port: port, background: true)
@@ -108,8 +93,7 @@ module Selenium
10893
expect(File).to have_received(:exist?).with('selenium_server_deploy.jar')
10994
expect(WebDriver::ChildProcess).to have_received(:build)
11095
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone',
111-
'--port', port.to_s, '--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s,
112-
'foo', 'bar')
96+
'--port', port.to_s, 'foo', 'bar')
11397
end
11498

11599
it 'adds additional JAVA options args' do
@@ -120,8 +104,6 @@ module Selenium
120104
'-jar', 'selenium_server_deploy.jar',
121105
'standalone',
122106
'--port', port.to_s,
123-
'--override-max-sessions', 'true',
124-
'--max-sessions', Etc.nprocessors.to_s,
125107
'foo',
126108
'bar')
127109
.and_return(mock_process)
@@ -140,8 +122,6 @@ module Selenium
140122
'-jar', 'selenium_server_deploy.jar',
141123
'standalone',
142124
'--port', port.to_s,
143-
'--override-max-sessions', 'true',
144-
'--max-sessions', Etc.nprocessors.to_s,
145125
'foo',
146126
'bar')
147127
end
@@ -214,8 +194,7 @@ module Selenium
214194
it 'raises Selenium::Server::Error if the server is not launched within the timeout' do
215195
allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
216196
allow(WebDriver::ChildProcess).to receive(:build)
217-
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s,
218-
'--override-max-sessions', 'true', '--max-sessions', Etc.nprocessors.to_s)
197+
.with('java', '-jar', 'selenium_server_deploy.jar', 'standalone', '--port', port.to_s)
219198
.and_return(mock_process)
220199

221200
poller = instance_double(WebDriver::SocketPoller)

0 commit comments

Comments
 (0)