Skip to content

Commit 791b5f4

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

File tree

4 files changed

+60
-82
lines changed

4 files changed

+60
-82
lines changed

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

Lines changed: 52 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,9 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
368368
configList.stream()
369369
.map(config -> config.get("display-name"))
370370
.collect(Collectors.toList());
371-
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
371+
boolean hasExplicitMaxSessions = nodeMaxSessions > DEFAULT_MAX_SESSIONS;
372+
Map<String, Integer> sessionsPerDriver =
373+
calculateOptimizedCpuDistribution(driverNames, false, hasExplicitMaxSessions);
372374

373375
for (Map<String, String> configMap : configList) {
374376
String displayName = configMap.get("display-name");
@@ -422,7 +424,9 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
422424
detectedDrivers.stream()
423425
.map(WebDriverInfo::getDisplayName)
424426
.collect(Collectors.toList());
425-
Map<String, Integer> sessionsPerDriver = calculateOptimizedCpuDistribution(driverNames);
427+
boolean hasExplicitMaxSessions = nodeMaxSessions > DEFAULT_MAX_SESSIONS;
428+
Map<String, Integer> sessionsPerDriver =
429+
calculateOptimizedCpuDistribution(driverNames, true, hasExplicitMaxSessions);
426430

427431
// Check if node max-sessions is explicitly set and within allowed range
428432
if (nodeMaxSessions != DEFAULT_MAX_SESSIONS) {
@@ -446,44 +450,59 @@ private Map<String, Integer> calculateActualMaxSessionsPerDriverConfig() {
446450
return result;
447451
}
448452

449-
private Map<String, Integer> calculateOptimizedCpuDistribution(List<String> driverNames) {
453+
private Map<String, Integer> calculateOptimizedCpuDistribution(
454+
List<String> driverNames, boolean detectDrivers, boolean hasExplicitMaxSessions) {
450455
Map<String, Integer> sessionsPerDriver = new HashMap<>();
451456

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

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();
484+
// Then distribute remaining cores among flexible drivers
485+
if (!flexibleDrivers.isEmpty() && remainingCores > 0) {
486+
int sessionsPerFlexibleDriver = Math.max(1, remainingCores / flexibleDrivers.size());
487+
int remainderCores = remainingCores % flexibleDrivers.size();
470488

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;
489+
// Distribute base sessions to all flexible drivers
490+
for (int i = 0; i < flexibleDrivers.size(); i++) {
491+
String driverName = flexibleDrivers.get(i);
492+
int sessions = sessionsPerFlexibleDriver;
475493

476-
// Distribute remainder cores to the first 'remainderCores' drivers
477-
if (i < remainderCores) {
478-
sessions++;
479-
}
494+
// Distribute remainder cores to the first 'remainderCores' drivers
495+
if (i < remainderCores) {
496+
sessions++;
497+
}
480498

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);
499+
sessionsPerDriver.put(driverName, sessions);
500+
}
501+
} else if (!flexibleDrivers.isEmpty()) {
502+
// No remaining cores, give each flexible driver 1 session
503+
for (String driverName : flexibleDrivers) {
504+
sessionsPerDriver.put(driverName, 1);
505+
}
487506
}
488507
}
489508

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)