Skip to content

Commit 2bd8a98

Browse files
GunniBuschGunniBusch
committed
Shard dependent test-bot runs
1 parent 1b01afd commit 2bd8a98

4 files changed

Lines changed: 279 additions & 22 deletions

File tree

‎Library/Homebrew/github_runner_matrix.rb‎

Lines changed: 120 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ 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+
@compatible_untested_dependent_names = T.let({}, T::Hash[GitHubRunner, T::Array[String]])
6768

6869
@runners = T.let([], T::Array[GitHubRunner])
6970
generate_runners!
@@ -73,19 +74,41 @@ def initialize(testing_formulae, deleted_formulae, all_supported:, dependent_mat
7374

7475
sig { returns(T::Array[RunnerSpecHash]) }
7576
def active_runner_specs_hash
76-
runners.select(&:active)
77-
.map(&:spec)
78-
.map(&:to_h)
77+
runners.select(&:active).flat_map do |runner|
78+
selected_runner_count = selected_runner_count_for(runner)
79+
runner_spec_hash = runner.spec.to_h
80+
name = runner_spec_hash.fetch(:name)
81+
82+
Array.new(selected_runner_count) do |dependent_shard_index|
83+
runner_spec_hash.merge(
84+
name: (
85+
if selected_runner_count > 1
86+
"#{name} (shard #{dependent_shard_index + 1}/#{selected_runner_count})"
87+
else
88+
name
89+
end
90+
),
91+
dependent_shard_index: dependent_shard_index,
92+
dependent_shard_total: selected_runner_count,
93+
)
94+
end
95+
end
7996
end
8097

8198
private
8299

83100
SELF_HOSTED_LINUX_RUNNER = "linux-self-hosted-1"
101+
DEPS_SHARDING_ENV = "HOMEBREW_DEPS_SHARDING"
102+
DEPS_SHARD_MAX_RUNNERS_ENV = "HOMEBREW_DEPS_SHARD_MAX_RUNNERS"
103+
DEPS_SHARD_BASE_THRESHOLD_ENV = "HOMEBREW_DEPS_SHARD_BASE_THRESHOLD"
104+
DEPS_SHARD_RUNNER_PENALTY_ENV = "HOMEBREW_DEPS_SHARD_RUNNER_PENALTY"
84105
# ARM macOS timeout, keep this under 1/2 of GitHub's job execution time limit for self-hosted runners.
85106
# https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#usage-limits
86107
GITHUB_ACTIONS_LONG_TIMEOUT = 2160 # 36 hours
87108
GITHUB_ACTIONS_SHORT_TIMEOUT = 60
88-
private_constant :SELF_HOSTED_LINUX_RUNNER, :GITHUB_ACTIONS_LONG_TIMEOUT, :GITHUB_ACTIONS_SHORT_TIMEOUT
109+
private_constant :SELF_HOSTED_LINUX_RUNNER, :DEPS_SHARDING_ENV, :DEPS_SHARD_MAX_RUNNERS_ENV,
110+
:DEPS_SHARD_BASE_THRESHOLD_ENV, :DEPS_SHARD_RUNNER_PENALTY_ENV,
111+
:GITHUB_ACTIONS_LONG_TIMEOUT, :GITHUB_ACTIONS_SHORT_TIMEOUT
89112

90113
sig { params(arch: Symbol).returns(LinuxRunnerSpec) }
91114
def linux_runner_spec(arch)
@@ -261,6 +284,71 @@ def generate_runners!
261284
@runners.freeze
262285
end
263286

287+
sig { params(runner: GitHubRunner).returns(Integer) }
288+
def selected_runner_count_for(runner)
289+
return 1 unless @dependent_matrix
290+
return 1 unless %w[1 true].include?(ENV.fetch(DEPS_SHARDING_ENV, "false").downcase)
291+
292+
max_available_runners = T.must(sharding_integer_env(DEPS_SHARD_MAX_RUNNERS_ENV, runner, 1))
293+
raise ArgumentError, "#{DEPS_SHARD_MAX_RUNNERS_ENV} must be positive" if max_available_runners <= 0
294+
return 1 if max_available_runners <= 1
295+
296+
base_spillover_threshold = sharding_integer_env(DEPS_SHARD_BASE_THRESHOLD_ENV, runner, nil)
297+
additional_runner_reluctance = sharding_integer_env(DEPS_SHARD_RUNNER_PENALTY_ENV, runner, nil)
298+
return 1 if base_spillover_threshold.nil? || additional_runner_reluctance.nil?
299+
300+
raise ArgumentError, "#{DEPS_SHARD_BASE_THRESHOLD_ENV} must be positive" if base_spillover_threshold <= 0
301+
if additional_runner_reluctance.negative?
302+
raise ArgumentError, "#{DEPS_SHARD_RUNNER_PENALTY_ENV} must be non-negative"
303+
end
304+
305+
dependent_count = compatible_untested_dependent_names(runner).count
306+
return 1 if dependent_count.zero?
307+
308+
selected_runner_count = if additional_runner_reluctance.zero?
309+
(dependent_count.to_f / base_spillover_threshold).ceil
310+
else
311+
quadratic_term = 4 * additional_runner_reluctance * dependent_count
312+
linear_term = (base_spillover_threshold - additional_runner_reluctance)**2
313+
numerator = -(base_spillover_threshold - additional_runner_reluctance) +
314+
Math.sqrt((linear_term + quadratic_term).to_f)
315+
denominator = 2 * additional_runner_reluctance
316+
(numerator / denominator).ceil
317+
end
318+
319+
selected_runner_count.clamp(1, max_available_runners)
320+
end
321+
322+
sig { params(base_env_name: String, runner: GitHubRunner, default: T.nilable(Integer)).returns(T.nilable(Integer)) }
323+
def sharding_integer_env(base_env_name, runner, default)
324+
platform = runner.platform.to_s.upcase
325+
arch = runner.arch.to_s.upcase
326+
version = runner.macos_version&.to_sym
327+
version = version.to_s.upcase if version
328+
329+
[
330+
("#{base_env_name}_#{platform}_#{arch}_#{version}" if version),
331+
"#{base_env_name}_#{platform}_#{arch}",
332+
"#{base_env_name}_#{platform}",
333+
base_env_name,
334+
].compact.each do |env_name|
335+
env_value = integer_env(env_name, nil)
336+
return env_value unless env_value.nil?
337+
end
338+
339+
default
340+
end
341+
342+
sig { params(env_name: String, default: T.nilable(Integer)).returns(T.nilable(Integer)) }
343+
def integer_env(env_name, default)
344+
env_value = ENV.fetch(env_name, nil)
345+
return default if env_value.blank?
346+
347+
Integer(env_value, 10)
348+
rescue ArgumentError
349+
raise ArgumentError, "#{env_name} must be an integer"
350+
end
351+
264352
sig { params(runner: GitHubRunner).returns(T::Array[String]) }
265353
def testable_formulae(runner)
266354
formulae = if @dependent_matrix
@@ -301,26 +389,37 @@ def compatible_testing_formulae(runner)
301389

302390
sig { params(runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) }
303391
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
392+
@formulae_with_untested_dependents[runner] ||= compatible_testing_formulae(runner).select do |formula|
393+
compatible_untested_dependent_names_for_formula(formula, runner).present?
394+
end
395+
end
308396

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)
397+
sig { params(runner: GitHubRunner).returns(T::Array[String]) }
398+
def compatible_untested_dependent_names(runner)
399+
@compatible_untested_dependent_names[runner] ||= compatible_testing_formulae(runner).flat_map do |formula|
400+
compatible_untested_dependent_names_for_formula(formula, runner)
401+
end.uniq.sort
402+
end
315403

316-
simulated_dependent_f.public_send(:"#{platform}_compatible?") &&
317-
simulated_dependent_f.public_send(:"#{arch}_compatible?")
318-
end
319-
end
404+
sig { params(formula: TestRunnerFormula, runner: GitHubRunner).returns(T::Array[String]) }
405+
def compatible_untested_dependent_names_for_formula(formula, runner)
406+
compatible_dependents_for_runner(formula, runner).map(&:name) - @testing_formulae.map(&:name)
407+
end
408+
409+
sig { params(formula: TestRunnerFormula, runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) }
410+
def compatible_dependents_for_runner(formula, runner)
411+
platform = runner.platform
412+
arch = runner.arch
413+
macos_version = runner.macos_version
414+
415+
formula.dependents(platform:, arch:, macos_version: macos_version&.to_sym)
416+
.select do |dependent_formula|
417+
Homebrew::SimulateSystem.with(os: platform, arch: Homebrew::SimulateSystem.arch_symbols.fetch(arch)) do
418+
simulated_dependent_formula = TestRunnerFormula.new(Formulary.factory(dependent_formula.name))
419+
next false if macos_version && !simulated_dependent_formula.compatible_with?(macos_version)
320420

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?
421+
simulated_dependent_formula.public_send(:"#{platform}_compatible?") &&
422+
simulated_dependent_formula.public_send(:"#{arch}_compatible?")
324423
end
325424
end
326425
end

‎Library/Homebrew/test/github_runner_matrix_spec.rb‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,46 @@
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+
allow(ENV).to receive(:fetch).with("HOMEBREW_DEPS_SHARDING", "false").and_return("1")
305+
allow(ENV).to receive(:fetch)
306+
.with("HOMEBREW_DEPS_SHARD_BASE_THRESHOLD_LINUX_X86_64", nil)
307+
.and_return("1")
308+
allow(ENV).to receive(:fetch)
309+
.with("HOMEBREW_DEPS_SHARD_BASE_THRESHOLD_LINUX", nil)
310+
.and_return("10")
311+
allow(ENV).to receive(:fetch)
312+
.with("HOMEBREW_DEPS_SHARD_RUNNER_PENALTY_LINUX_X86_64", nil)
313+
.and_return(nil)
314+
allow(ENV).to receive(:fetch)
315+
.with("HOMEBREW_DEPS_SHARD_RUNNER_PENALTY_LINUX", nil)
316+
.and_return(nil)
317+
allow(ENV).to receive(:fetch)
318+
.with("HOMEBREW_DEPS_SHARD_RUNNER_PENALTY", nil)
319+
.and_return("0")
320+
allow(ENV).to receive(:fetch)
321+
.with("HOMEBREW_DEPS_SHARD_MAX_RUNNERS_LINUX_X86_64", nil)
322+
.and_return(nil)
323+
allow(ENV).to receive(:fetch)
324+
.with("HOMEBREW_DEPS_SHARD_MAX_RUNNERS_LINUX", nil)
325+
.and_return("2")
326+
327+
matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true)
328+
329+
expect(matrix.active_runner_specs_hash.map { |spec| spec.fetch(:name) }).to eq([
330+
"Linux x86_64 (shard 1/2)",
331+
"Linux x86_64 (shard 2/2)",
332+
])
333+
expect(matrix.active_runner_specs_hash.map { |spec| spec.fetch(:dependent_shard_index) }).to eq([0, 1])
334+
end
295335
end
296336

297337
context "when dependent formulae require macOS" do

‎Library/Homebrew/test/test_bot/formulae_spec.rb‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,27 @@
4646
end
4747
end
4848
end
49+
50+
describe Homebrew::TestBot::FormulaeDependents do
51+
let(:formulae_dependents) do
52+
described_class.new(tap: nil, git: "git", dry_run: true, fail_fast: false, verbose: false)
53+
end
54+
55+
it "partitions and filters shard dependents deterministically" do
56+
alpha = instance_double(Formula, full_name: "alpha")
57+
bravo = instance_double(Formula, full_name: "bravo")
58+
59+
first_shard = formulae_dependents.send(
60+
:sharded_dependent_formula_names,
61+
%w[alpha bravo charlie delta echo],
62+
dependent_shard_index: 0,
63+
dependent_shard_total: 2,
64+
)
65+
formulae_dependents.instance_variable_set(:@assigned_dependent_formula_names, Set.new(first_shard))
66+
formulae_dependents.instance_variable_set(:@handled_dependent_formula_names, Set.new(["alpha"]))
67+
68+
expect(first_shard).to eq(%w[alpha charlie echo])
69+
expect(formulae_dependents.send(:sharded_dependents, [alpha, bravo])).to eq([])
70+
end
71+
end
4972
end

‎Library/Homebrew/test_bot/formulae_dependents.rb‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
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"
9+
710
sig { params(testing_formulae: T::Array[String]).returns(T::Array[String]) }
811
attr_writer :testing_formulae
912

@@ -24,6 +27,8 @@ def initialize(tap:, git:, dry_run:, fail_fast:, verbose:)
2427
@testing_formulae_with_tested_dependents = T.let([], T::Array[String])
2528
@tested_dependents_list = T.let(nil, T.nilable(Pathname))
2629
@dependent_testing_formulae = T.let([], T::Array[String])
30+
@assigned_dependent_formula_names = T.let(nil, T.nilable(T::Set[String]))
31+
@handled_dependent_formula_names = T.let(Set.new, T::Set[String])
2732
end
2833

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

4348
@dependent_testing_formulae = sorted_formulae - skipped_or_failed_formulae
49+
configure_dependent_sharding!(args:)
4450

4551
install_formulae_if_needed_from_bottles!(installable_bottles, args:)
4652

@@ -145,6 +151,12 @@ def dependent_formulae!(formula_name, args:)
145151
bottled_dependents.each do |dependent|
146152
install_dependent(dependent, testable_dependents, args:)
147153
end
154+
155+
return if @assigned_dependent_formula_names.nil?
156+
157+
(source_dependents + bottled_dependents).each do |dependent|
158+
@handled_dependent_formula_names.add(dependent.full_name)
159+
end
148160
end
149161

150162
sig {
@@ -184,7 +196,8 @@ def dependents_for_formula(formula, formula_name, args:)
184196
dependents.sort!
185197

186198
dependents -= @tested_formulae
187-
dependents = dependents.map { |d| Formulary.factory(d) }
199+
dependents = dependents.map { |dependent_formula_name| Formulary.factory(dependent_formula_name) }
200+
dependents = sharded_dependents(dependents)
188201

189202
dependents = dependents.zip(dependents.map do |f|
190203
if skip_recursive_dependents
@@ -256,6 +269,88 @@ def dependents_for_formula(formula, formula_name, args:)
256269
[source_dependents, bottled_dependents, testable_dependents]
257270
end
258271

272+
sig { params(args: Homebrew::Cmd::TestBotCmd::Args).void }
273+
def configure_dependent_sharding!(args:)
274+
dependent_shard_index = integer_env(DEPS_SHARD_INDEX_ENV, 0)
275+
dependent_shard_total = integer_env(DEPS_SHARD_TOTAL_ENV, 1)
276+
raise ArgumentError, "#{DEPS_SHARD_TOTAL_ENV} must be positive" if dependent_shard_total <= 0
277+
278+
if dependent_shard_index.negative? || dependent_shard_index >= dependent_shard_total
279+
raise ArgumentError,
280+
"#{DEPS_SHARD_INDEX_ENV} must be between 0 and #{dependent_shard_total - 1}"
281+
end
282+
283+
@assigned_dependent_formula_names = nil
284+
@handled_dependent_formula_names.clear
285+
return if dependent_shard_total == 1
286+
287+
all_dependent_formula_names = @dependent_testing_formulae.flat_map do |formula_name|
288+
formula = Formulary.factory(formula_name)
289+
uses_args = %w[--formula --eval-all]
290+
uses_include_test_args = [*uses_args, "--include-test"]
291+
# Always skip recursive dependents on Intel. It's really slow.
292+
# Also skip recursive dependents on Linux unless it's a Linux-only formula.
293+
#
294+
# TODO: move to extend/os
295+
# rubocop:todo Homebrew/MoveToExtendOS
296+
skip_recursive_dependents = args.skip_recursive_dependents? ||
297+
(OS.mac? && Hardware::CPU.intel?) ||
298+
(OS.linux? && formula.requirements.exclude?(LinuxRequirement.new))
299+
# rubocop:enable Homebrew/MoveToExtendOS
300+
uses_include_test_args << "--recursive" unless skip_recursive_dependents
301+
302+
dependent_formula_names = with_env(HOMEBREW_STDERR: "1") do
303+
Utils.safe_popen_read("brew", "uses", *uses_include_test_args, formula_name)
304+
.split("\n")
305+
end
306+
dependent_formula_names + with_env(HOMEBREW_STDERR: "1") do
307+
Utils.safe_popen_read("brew", "uses", *uses_args, "--include-build", formula_name)
308+
.split("\n")
309+
end
310+
end.uniq.sort
311+
@assigned_dependent_formula_names = Set.new(
312+
sharded_dependent_formula_names(
313+
all_dependent_formula_names,
314+
dependent_shard_index:,
315+
dependent_shard_total:,
316+
),
317+
)
318+
end
319+
320+
sig { params(dependents: T::Array[Formula]).returns(T::Array[Formula]) }
321+
def sharded_dependents(dependents)
322+
assigned_dependent_formula_names = @assigned_dependent_formula_names
323+
return dependents if assigned_dependent_formula_names.nil?
324+
325+
dependents.select do |dependent|
326+
assigned_dependent_formula_names.include?(dependent.full_name) &&
327+
@handled_dependent_formula_names.exclude?(dependent.full_name)
328+
end
329+
end
330+
331+
sig {
332+
params(
333+
dependent_formula_names: T::Array[String],
334+
dependent_shard_index: Integer,
335+
dependent_shard_total: Integer,
336+
).returns(T::Array[String])
337+
}
338+
def sharded_dependent_formula_names(dependent_formula_names, dependent_shard_index:, dependent_shard_total:)
339+
dependent_formula_names.each_with_index.filter_map do |dependent_formula_name, dependent_position|
340+
dependent_formula_name if dependent_position % dependent_shard_total == dependent_shard_index
341+
end
342+
end
343+
344+
sig { params(env_name: String, default: Integer).returns(Integer) }
345+
def integer_env(env_name, default)
346+
env_value = ENV.fetch(env_name, nil)
347+
return default if env_value.blank?
348+
349+
Integer(env_value, 10)
350+
rescue ArgumentError
351+
raise ArgumentError, "#{env_name} must be an integer"
352+
end
353+
259354
sig {
260355
params(
261356
dependent: Formula,

0 commit comments

Comments
 (0)