Skip to content

Commit fe25bbf

Browse files
committed
test-bot: shard dependent runs across runners
1 parent 1b01afd commit fe25bbf

File tree

4 files changed

+199
-32
lines changed

4 files changed

+199
-32
lines changed

Library/Homebrew/github_runner_matrix.rb

Lines changed: 94 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -64,28 +64,40 @@ def initialize(testing_formulae, deleted_formulae, all_supported:, dependent_mat
6464
@dependent_matrix = dependent_matrix
6565
@compatible_testing_formulae = T.let({}, T::Hash[GitHubRunner, T::Array[TestRunnerFormula]])
6666
@formulae_with_untested_dependents = T.let({}, T::Hash[GitHubRunner, T::Array[TestRunnerFormula]])
67-
67+
@compatible_untested_dependent_names = T.let({}, T::Hash[GitHubRunner, T::Array[String]])
6868
@runners = T.let([], T::Array[GitHubRunner])
6969
generate_runners!
7070

7171
freeze
7272
end
7373

74-
sig { returns(T::Array[RunnerSpecHash]) }
74+
sig { returns(T::Array[T::Hash[Symbol, T.untyped]]) }
7575
def active_runner_specs_hash
76-
runners.select(&:active)
77-
.map(&:spec)
78-
.map(&:to_h)
76+
runners.filter(&:active).flat_map do |r|
77+
Array.new(shard_count = selected_runner_count_for(r)) do |i|
78+
(spec = r.spec.to_h).merge(
79+
name: (shard_count > 1) ? "#{spec[:name]} (shard #{i + 1}/#{shard_count})" : spec[:name],
80+
shard_index: i,
81+
shard_total: shard_count,
82+
)
83+
end
84+
end
7985
end
8086

8187
private
8288

8389
SELF_HOSTED_LINUX_RUNNER = "linux-self-hosted-1"
90+
DEPS_SHARDING_ENV = "HOMEBREW_DEPS_SHARDING"
91+
DEPS_SHARD_MAX_RUNNERS_ENV = "HOMEBREW_DEPS_SHARD_MAX_RUNNERS"
92+
DEPS_SHARD_BASE_THRESHOLD_ENV = "HOMEBREW_DEPS_SHARD_BASE_THRESHOLD"
93+
DEPS_SHARD_RUNNER_PENALTY_ENV = "HOMEBREW_DEPS_SHARD_RUNNER_PENALTY"
8494
# ARM macOS timeout, keep this under 1/2 of GitHub's job execution time limit for self-hosted runners.
8595
# https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#usage-limits
8696
GITHUB_ACTIONS_LONG_TIMEOUT = 2160 # 36 hours
8797
GITHUB_ACTIONS_SHORT_TIMEOUT = 60
88-
private_constant :SELF_HOSTED_LINUX_RUNNER, :GITHUB_ACTIONS_LONG_TIMEOUT, :GITHUB_ACTIONS_SHORT_TIMEOUT
98+
private_constant :SELF_HOSTED_LINUX_RUNNER, :DEPS_SHARDING_ENV, :DEPS_SHARD_MAX_RUNNERS_ENV,
99+
:DEPS_SHARD_BASE_THRESHOLD_ENV, :DEPS_SHARD_RUNNER_PENALTY_ENV,
100+
:GITHUB_ACTIONS_LONG_TIMEOUT, :GITHUB_ACTIONS_SHORT_TIMEOUT
89101

90102
sig { params(arch: Symbol).returns(LinuxRunnerSpec) }
91103
def linux_runner_spec(arch)
@@ -261,6 +273,62 @@ def generate_runners!
261273
@runners.freeze
262274
end
263275

276+
sig { params(runner: GitHubRunner).returns(Integer) }
277+
def selected_runner_count_for(runner)
278+
return 1 if !@dependent_matrix || %w[1 true].exclude?(ENV.fetch(DEPS_SHARDING_ENV, "false").downcase)
279+
280+
max_available_runners = T.must(sharding_integer_env(DEPS_SHARD_MAX_RUNNERS_ENV, runner, 1))
281+
base_spillover_threshold = sharding_integer_env(DEPS_SHARD_BASE_THRESHOLD_ENV, runner, nil)
282+
additional_runner_reluctance = sharding_integer_env(DEPS_SHARD_RUNNER_PENALTY_ENV, runner, nil)
283+
raise ArgumentError, "#{DEPS_SHARD_MAX_RUNNERS_ENV} must be positive" unless max_available_runners.positive?
284+
raise ArgumentError, "#{DEPS_SHARD_BASE_THRESHOLD_ENV} must be positive" if base_spillover_threshold&.<= 0
285+
286+
if additional_runner_reluctance&.negative?
287+
raise ArgumentError,
288+
"#{DEPS_SHARD_RUNNER_PENALTY_ENV} must be non-negative"
289+
end
290+
return 1 if max_available_runners <= 1 || base_spillover_threshold.nil? || additional_runner_reluctance.nil?
291+
292+
dependent_names = @compatible_untested_dependent_names[runner] ||= compatible_testing_formulae(runner)
293+
.flat_map do |formula|
294+
compatible_untested_dependent_names_for_formula(
295+
formula, runner
296+
)
297+
end.uniq.sort
298+
dependent_count = dependent_names.count
299+
return 1 if dependent_count.zero?
300+
301+
runner_count = if additional_runner_reluctance.zero?
302+
dependent_count.to_f / base_spillover_threshold
303+
else
304+
threshold_difference = base_spillover_threshold - additional_runner_reluctance
305+
discriminant = (threshold_difference**2) + (4 * additional_runner_reluctance * dependent_count)
306+
(Math.sqrt(discriminant.to_f) - threshold_difference) / (2 * additional_runner_reluctance)
307+
end
308+
309+
runner_count.ceil.clamp(1, max_available_runners)
310+
end
311+
312+
sig { params(base_env_name: String, runner: GitHubRunner, default: T.nilable(Integer)).returns(T.nilable(Integer)) }
313+
def sharding_integer_env(base_env_name, runner, default)
314+
platform = runner.platform.to_s.upcase
315+
arch = runner.arch.to_s.upcase
316+
version = runner.macos_version&.to_sym
317+
version = version.to_s.upcase if version
318+
319+
[
320+
("#{base_env_name}_#{platform}_#{arch}_#{version}" if version),
321+
"#{base_env_name}_#{platform}_#{arch}",
322+
"#{base_env_name}_#{platform}",
323+
base_env_name,
324+
].compact.each do |env_name|
325+
env_value = ENV.fetch(env_name, nil)
326+
return Integer(env_value, 10) if env_value.present?
327+
end
328+
329+
default
330+
end
331+
264332
sig { params(runner: GitHubRunner).returns(T::Array[String]) }
265333
def testable_formulae(runner)
266334
formulae = if @dependent_matrix
@@ -301,27 +369,30 @@ def compatible_testing_formulae(runner)
301369

302370
sig { params(runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) }
303371
def formulae_with_untested_dependents(runner)
304-
@formulae_with_untested_dependents[runner] ||= begin
305-
platform = runner.platform
306-
arch = runner.arch
307-
macos_version = runner.macos_version
372+
@formulae_with_untested_dependents[runner] ||= compatible_testing_formulae(runner).select do |formula|
373+
compatible_untested_dependent_names_for_formula(formula, runner).present?
374+
end
375+
end
308376

309-
compatible_testing_formulae(runner).select do |formula|
310-
compatible_dependents = formula.dependents(platform:, arch:, macos_version: macos_version&.to_sym)
311-
.select do |dependent_f|
312-
Homebrew::SimulateSystem.with(os: platform, arch: Homebrew::SimulateSystem.arch_symbols.fetch(arch)) do
313-
simulated_dependent_f = TestRunnerFormula.new(Formulary.factory(dependent_f.name))
314-
next false if macos_version && !simulated_dependent_f.compatible_with?(macos_version)
377+
sig { params(formula: TestRunnerFormula, runner: GitHubRunner).returns(T::Array[String]) }
378+
def compatible_untested_dependent_names_for_formula(formula, runner)
379+
platform = runner.platform
380+
arch = runner.arch
381+
macos_version = runner.macos_version
315382

316-
simulated_dependent_f.public_send(:"#{platform}_compatible?") &&
317-
simulated_dependent_f.public_send(:"#{arch}_compatible?")
318-
end
319-
end
383+
compatible_dependents = formula.dependents(platform:, arch:, macos_version: macos_version&.to_sym)
384+
.select do |dependent_f|
385+
Homebrew::SimulateSystem.with(os: platform, arch: Homebrew::SimulateSystem.arch_symbols.fetch(arch)) do
386+
simulated_dependent_f = TestRunnerFormula.new(Formulary.factory(dependent_f.name))
387+
next false if macos_version && !simulated_dependent_f.compatible_with?(macos_version)
320388

321-
# These arrays will generally have been generated by different Formulary caches,
322-
# so we can only compare them by name and not directly.
323-
(compatible_dependents.map(&:name) - @testing_formulae.map(&:name)).present?
389+
simulated_dependent_f.public_send(:"#{platform}_compatible?") &&
390+
simulated_dependent_f.public_send(:"#{arch}_compatible?")
324391
end
325392
end
393+
394+
# These arrays will generally have been generated by different Formulary caches,
395+
# so we can only compare them by name and not directly.
396+
compatible_dependents.map(&:name) - @testing_formulae.map(&:name)
326397
end
327398
end

Library/Homebrew/test/github_runner_matrix_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,34 @@
292292
matrix = described_class.new([testball], ["deleted"], all_supported: false, dependent_matrix: true)
293293
expect(get_runner_names(matrix)).to eq(["Linux x86_64"])
294294
end
295+
296+
it "shards dependent runners using the most specific sharding settings" do
297+
allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true)
298+
allow(Formula).to receive(:all).and_return([
299+
testball,
300+
testball_depender_linux,
301+
setup_test_runner_formula("testball-depender-linux-two", ["testball", :linux]),
302+
].map(&:formula))
303+
allow(ENV).to receive(:[]).with("HOMEBREW_LINUX_RUNNER").and_return("linux-self-hosted-1")
304+
env_fetch_overrides = {
305+
["HOMEBREW_DEPS_SHARDING", "false"] => "1",
306+
["HOMEBREW_DEPS_SHARD_BASE_THRESHOLD_LINUX_X86_64", nil] => "1",
307+
["HOMEBREW_DEPS_SHARD_BASE_THRESHOLD_LINUX", nil] => "10",
308+
["HOMEBREW_DEPS_SHARD_RUNNER_PENALTY", nil] => "0",
309+
["HOMEBREW_DEPS_SHARD_MAX_RUNNERS_LINUX", nil] => "2",
310+
}
311+
allow(ENV).to receive(:fetch).and_wrap_original do |original, key, default = nil|
312+
if env_fetch_overrides.key?([key, default])
313+
env_fetch_overrides[[key, default]]
314+
else
315+
original.call(key, default)
316+
end
317+
end
318+
matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true)
319+
320+
expect(matrix.active_runner_specs_hash.map { |spec| spec.fetch(:name) })
321+
.to eq(["Linux x86_64 (shard 1/2)", "Linux x86_64 (shard 2/2)"])
322+
end
295323
end
296324

297325
context "when dependent formulae require macOS" do

Library/Homebrew/test/test_bot/formulae_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,12 @@
4646
end
4747
end
4848
end
49+
50+
it "skips handled formulae from the assigned shard" do
51+
deps = Homebrew::TestBot::FormulaeDependents.new(tap: nil, git: "git", dry_run: true, fail_fast: false,
52+
verbose: false)
53+
deps.instance_variable_set(:@assigned_dependent_formula_names, Set["alpha"])
54+
deps.instance_variable_set(:@handled_dependent_formula_names, Set["alpha"])
55+
expect(deps.send(:sharded_dependents, [instance_double(Formula, full_name: "alpha")])).to eq([])
56+
end
4957
end

Library/Homebrew/test_bot/formulae_dependents.rb

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
module Homebrew
55
module TestBot
66
class FormulaeDependents < TestFormulae
7+
DEPS_SHARD_INDEX_ENV = "HOMEBREW_DEPS_SHARD_INDEX"
8+
DEPS_SHARD_TOTAL_ENV = "HOMEBREW_DEPS_SHARD_TOTAL"
79
sig { params(testing_formulae: T::Array[String]).returns(T::Array[String]) }
810
attr_writer :testing_formulae
911

@@ -24,6 +26,8 @@ def initialize(tap:, git:, dry_run:, fail_fast:, verbose:)
2426
@testing_formulae_with_tested_dependents = T.let([], T::Array[String])
2527
@tested_dependents_list = T.let(nil, T.nilable(Pathname))
2628
@dependent_testing_formulae = T.let([], T::Array[String])
29+
@assigned_dependent_formula_names = T.let(nil, T.nilable(T::Set[String]))
30+
@handled_dependent_formula_names = T.let(Set.new, T::Set[String])
2731
end
2832

2933
sig { params(args: Homebrew::Cmd::TestBotCmd::Args).void }
@@ -41,6 +45,7 @@ def run!(args:)
4145
@tested_dependents_list = Pathname("tested-dependents-#{Utils::Bottles.tag}.txt")
4246

4347
@dependent_testing_formulae = sorted_formulae - skipped_or_failed_formulae
48+
configure_dependent_sharding!(args:)
4449

4550
install_formulae_if_needed_from_bottles!(installable_bottles, args:)
4651

@@ -145,33 +150,41 @@ def dependent_formulae!(formula_name, args:)
145150
bottled_dependents.each do |dependent|
146151
install_dependent(dependent, testable_dependents, args:)
147152
end
153+
return if @assigned_dependent_formula_names.nil?
154+
155+
(source_dependents + bottled_dependents).each do |dependent|
156+
@handled_dependent_formula_names.add(dependent.full_name)
157+
end
148158
end
149159

150160
sig {
151-
params(formula: Formula, formula_name: String, args: Homebrew::Cmd::TestBotCmd::Args)
152-
.returns([T::Array[Formula], T::Array[Formula], T::Array[Formula]])
161+
params(formula: Formula, args: Homebrew::Cmd::TestBotCmd::Args)
162+
.returns(T::Boolean)
153163
}
154-
def dependents_for_formula(formula, formula_name, args:)
155-
info_header "Determining dependents..."
156-
164+
def skip_recursive_dependents_for(formula, args:)
157165
# Always skip recursive dependents on Intel. It's really slow.
158166
# Also skip recursive dependents on Linux unless it's a Linux-only formula.
159167
#
160168
# TODO: move to extend/os
161169
# rubocop:todo Homebrew/MoveToExtendOS
162-
skip_recursive_dependents = args.skip_recursive_dependents? ||
163-
(OS.mac? && Hardware::CPU.intel?) ||
164-
(OS.linux? && formula.requirements.exclude?(LinuxRequirement.new))
170+
args.skip_recursive_dependents? ||
171+
(OS.mac? && Hardware::CPU.intel?) ||
172+
(OS.linux? && formula.requirements.exclude?(LinuxRequirement.new))
165173
# rubocop:enable Homebrew/MoveToExtendOS
174+
end
166175

176+
sig {
177+
params(formula_name: String, skip_recursive_dependents: T::Boolean)
178+
.returns(T::Array[String])
179+
}
180+
def dependent_formula_names(formula_name, skip_recursive_dependents:)
167181
uses_args = %w[--formula --eval-all]
168182
uses_include_test_args = [*uses_args, "--include-test"]
169183
uses_include_test_args << "--recursive" unless skip_recursive_dependents
170184
dependents = with_env(HOMEBREW_STDERR: "1") do
171185
Utils.safe_popen_read("brew", "uses", *uses_include_test_args, formula_name)
172186
.split("\n")
173187
end
174-
175188
# TODO: Consider handling the following case better.
176189
# `foo` has a build dependency on `bar`, and `bar` has a runtime dependency on
177190
# `baz`. When testing `baz` with `--build-dependents-from-source`, `foo` is
@@ -182,9 +195,21 @@ def dependents_for_formula(formula, formula_name, args:)
182195
end
183196
dependents.uniq!
184197
dependents.sort!
198+
dependents
199+
end
185200

201+
sig {
202+
params(formula: Formula, formula_name: String, args: Homebrew::Cmd::TestBotCmd::Args)
203+
.returns([T::Array[Formula], T::Array[Formula], T::Array[Formula]])
204+
}
205+
def dependents_for_formula(formula, formula_name, args:)
206+
info_header "Determining dependents..."
207+
208+
skip_recursive_dependents = skip_recursive_dependents_for(formula, args:)
209+
dependents = dependent_formula_names(formula_name, skip_recursive_dependents:)
186210
dependents -= @tested_formulae
187211
dependents = dependents.map { |d| Formulary.factory(d) }
212+
dependents = sharded_dependents(dependents)
188213

189214
dependents = dependents.zip(dependents.map do |f|
190215
if skip_recursive_dependents
@@ -256,6 +281,41 @@ def dependents_for_formula(formula, formula_name, args:)
256281
[source_dependents, bottled_dependents, testable_dependents]
257282
end
258283

284+
sig { params(args: Homebrew::Cmd::TestBotCmd::Args).void }
285+
def configure_dependent_sharding!(args:)
286+
shard_index = (env_value = ENV.fetch(DEPS_SHARD_INDEX_ENV, nil)).blank? ? 0 : Integer(env_value, 10)
287+
shard_total = (env_value = ENV.fetch(DEPS_SHARD_TOTAL_ENV, nil)).blank? ? 1 : Integer(env_value, 10)
288+
raise ArgumentError, "#{DEPS_SHARD_TOTAL_ENV} must be positive" unless shard_total.positive?
289+
raise ArgumentError, "#{DEPS_SHARD_INDEX_ENV} must be between 0 and #{shard_total - 1}" unless
290+
(0...shard_total).cover?(shard_index)
291+
292+
@assigned_dependent_formula_names = nil
293+
@handled_dependent_formula_names.clear
294+
return if shard_total == 1
295+
296+
all_dependent_formula_names = @dependent_testing_formulae.flat_map do |formula_name|
297+
formula = Formulary.factory(formula_name)
298+
skip_recursive_dependents = skip_recursive_dependents_for(formula, args:)
299+
dependent_formula_names(formula_name, skip_recursive_dependents:)
300+
end.uniq.sort
301+
@assigned_dependent_formula_names = Set.new(
302+
all_dependent_formula_names.each_with_index.filter_map do |name, position|
303+
name if position % shard_total == shard_index
304+
end,
305+
)
306+
end
307+
308+
sig { params(dependents: T::Array[Formula]).returns(T::Array[Formula]) }
309+
def sharded_dependents(dependents)
310+
assigned_dependent_formula_names = @assigned_dependent_formula_names
311+
return dependents if assigned_dependent_formula_names.nil?
312+
313+
dependents.select do |dependent|
314+
assigned_dependent_formula_names.include?(dependent.full_name) &&
315+
@handled_dependent_formula_names.exclude?(dependent.full_name)
316+
end
317+
end
318+
259319
sig {
260320
params(
261321
dependent: Formula,

0 commit comments

Comments
 (0)