Skip to content

Commit 86365bb

Browse files
spiliopoulosclaude
andcommitted
ruby: Measure worker slack and let a sample of workers exit idle
A worker that has drained the queue does not exit. `exhausted?` is `queue_initialized? && size == 0`, and `size` counts pending plus running across the whole build, so every worker stays online until the slowest one finishes. That is deliberate, so a test that times out or gets requeued still has somewhere to run. It is not free. On a Figma permissions build of 20 workers over 141 files, two workers ran 105 and 34 tests and stopped 62 milliseconds apart. Wall-time profiles put roughly 22% of main thread time in the poll backoff. Two changes. Measurement. The worker records when its last test finished and exposes `slack_duration`, the gap between that and leaving the queue. The minitest adapter emits it as `minitests.queue.worker.slack`, tagged with whether the worker held requeue duty. Emitted from the adapter, not the queue, because ci/queue stays framework agnostic and Statsd lives under minitest/. Control. At init each worker draws a number and compares it to CI_QUEUE_IDLE_EXIT_PROBABILITY. Workers above the threshold keep today's behaviour. The rest leave once they have been idle for CI_QUEUE_IDLE_EXIT_GRACE, after the existing sleep and backoff. Defaults are 0.0 and 30, so nothing changes until the knob is set. The draw uses Random.new rather than Kernel#rand. The global RNG is seeded from --seed, which is identical across workers, so every worker would otherwise draw the same number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0370167 commit 86365bb

5 files changed

Lines changed: 133 additions & 6 deletions

File tree

‎ruby/lib/ci/queue/configuration.rb‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class Configuration
1616
attr_accessor :write_duration_averages
1717
attr_accessor :heartbeat_grace_period, :heartbeat_interval
1818
attr_accessor :retry_selection
19+
attr_accessor :idle_exit_probability, :idle_exit_grace
1920
attr_reader :circuit_breakers
2021
attr_writer :seed, :build_id
2122
attr_writer :queue_init_timeout, :report_timeout, :inactive_workers_timeout
@@ -31,6 +32,8 @@ def from_env(env)
3132
redis_ttl: env['CI_QUEUE_REDIS_TTL']&.to_i || 8 * 60 * 60,
3233
known_flaky_tests: load_known_flaky_tests(env['CI_QUEUE_KNOWN_FLAKY_TESTS']),
3334
branch: env['BUILDKITE_BRANCH'],
35+
idle_exit_probability: env['CI_QUEUE_IDLE_EXIT_PROBABILITY']&.to_f || 0.0,
36+
idle_exit_grace: env['CI_QUEUE_IDLE_EXIT_GRACE']&.to_f || 30.0,
3437
)
3538
end
3639

@@ -68,7 +71,9 @@ def initialize(
6871
timing_redis_url: nil,
6972
heartbeat_grace_period: 30,
7073
heartbeat_interval: 10,
71-
retry_selection: :failed_tests
74+
retry_selection: :failed_tests,
75+
idle_exit_probability: 0.0,
76+
idle_exit_grace: 30.0
7277
)
7378
@build_id = build_id
7479
@circuit_breakers = [CircuitBreaker::Disabled]
@@ -108,6 +113,8 @@ def initialize(
108113
@heartbeat_grace_period = heartbeat_grace_period
109114
@heartbeat_interval = heartbeat_interval
110115
@retry_selection = retry_selection
116+
@idle_exit_probability = idle_exit_probability
117+
@idle_exit_grace = idle_exit_grace
111118
end
112119

113120
def queue_init_timeout

‎ruby/lib/ci/queue/redis/worker.rb‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,30 @@ def initialize(redis, config)
2424
@reserved_tests = Concurrent::Set.new
2525
@shutdown_required = false
2626
@idle_since = nil
27+
@last_test_finished_at = nil
28+
# A worker with nothing left to reserve normally stays online until the whole
29+
# build drains, so it can pick up a test that times out or gets requeued. Only
30+
# a sample needs to do that. Random.new rather than Kernel#rand: the global RNG
31+
# is seeded from --seed, which is identical across workers, so every worker
32+
# would draw the same number.
33+
@waits_for_requeues = Random.new.rand > config.idle_exit_probability
2734
super(redis, config)
2835
end
2936

3037
attr_accessor :idle_since
38+
attr_reader :last_test_finished_at
39+
40+
def waits_for_requeues?
41+
@waits_for_requeues
42+
end
43+
44+
# Time between the last test finishing and the worker leaving the queue: capacity
45+
# the build paid for and did not use. Nil when the worker never reserved a test.
46+
def slack_duration
47+
return nil if @last_test_finished_at.nil?
48+
49+
CI::Queue.time_now - @last_test_finished_at
50+
end
3151

3252
def distributed?
3353
true
@@ -75,6 +95,15 @@ def idle?
7595
!@idle_since.nil?
7696
end
7797

98+
# Workers that did not draw the requeue duty leave once they have been idle for
99+
# the grace period, instead of waiting for the whole build to drain.
100+
def idle_exit?
101+
return false if @waits_for_requeues
102+
return false if @idle_since.nil?
103+
104+
CI::Queue.time_now - @idle_since >= config.idle_exit_grace
105+
end
106+
78107
def poll
79108
wait_for_master
80109
if master?
@@ -83,26 +112,28 @@ def poll
83112
master_id = master_worker_id
84113
warn "Worker #{worker_id} saw master worker: #{master_id}" if master_id
85114
end
86-
idle_since = nil
115+
@idle_since = nil
87116
idle_state_printed = false
88117
attempt = 0
89-
until shutdown_required? || config.circuit_breakers.any?(&:open?) || exhausted? || max_test_failed?
118+
until shutdown_required? || config.circuit_breakers.any?(&:open?) || exhausted? ||
119+
max_test_failed? || idle_exit?
90120
if id = reserve
91121
attempt = 0
92-
idle_since = nil
122+
@idle_since = nil
93123
executable = resolve_executable(id)
94124

95125
if executable
96126
with_heartbeat(id) do
97127
yield executable
98128
end
129+
@last_test_finished_at = CI::Queue.time_now
99130
else
100131
warn("Warning: Could not resolve executable for ID #{id.inspect}. Acknowledging to remove from queue.")
101132
acknowledge(id)
102133
end
103134
else
104-
idle_since ||= CI::Queue.time_now
105-
if CI::Queue.time_now - idle_since > 120 && !idle_state_printed
135+
@idle_since ||= CI::Queue.time_now
136+
if CI::Queue.time_now - @idle_since > 120 && !idle_state_printed
106137
puts "Worker #{worker_id} has been idle for 120 seconds. Printing global state..."
107138
running_tests = redis.zrange(key('running'), 0, -1, withscores: true)
108139
puts " Processed tests: #{redis.scard(key('processed'))}"

‎ruby/lib/minitest/queue.rb‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
require 'minitest/queue/build_status_recorder'
1010
require 'minitest/queue/build_status_reporter'
1111
require 'minitest/queue/order_reporter'
12+
require 'minitest/queue/statsd'
1213
require 'minitest/queue/junit_reporter'
1314
require 'minitest/queue/test_data_reporter'
1415
require 'minitest/queue/grind_recorder'
@@ -232,10 +233,34 @@ def run_from_queue(reporter, *)
232233
run_single_test(executable, reporter)
233234
end
234235
end
236+
report_worker_slack
235237
end
236238

237239
private
238240

241+
# How long this worker sat idle before leaving. Emitted here rather than from the
242+
# queue because ci/queue stays framework agnostic, and Statsd lives under minitest/.
243+
def report_worker_slack
244+
return unless queue.respond_to?(:slack_duration)
245+
246+
slack = queue.slack_duration
247+
return if slack.nil?
248+
249+
endpoint = queue.config.statsd_endpoint
250+
return if endpoint.nil?
251+
252+
statsd = Minitest::Queue::Statsd.new(
253+
addr: endpoint,
254+
namespace: 'minitests.queue',
255+
default_tags: ["slug:#{ENV['BUILDKITE_PROJECT_SLUG']}"],
256+
)
257+
statsd.measure(
258+
'worker.slack',
259+
slack * 1000,
260+
tags: ["waits_for_requeues:#{queue.waits_for_requeues?}"],
261+
)
262+
end
263+
239264
def run_chunk(chunk, reporter)
240265
@in_chunk_context = true
241266

‎ruby/test/ci/queue/configuration_test.rb‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ def test_cirleci_defaults
1717
assert_equal 'faa647bbb8168a77cf338e7488c3f8445c3e6554', config.seed
1818
end
1919

20+
def test_idle_exit_defaults_keep_every_worker_online
21+
config = Configuration.from_env({})
22+
assert_equal 0.0, config.idle_exit_probability
23+
assert_equal 30.0, config.idle_exit_grace
24+
end
25+
26+
def test_idle_exit_read_from_env
27+
config = Configuration.from_env(
28+
'CI_QUEUE_IDLE_EXIT_PROBABILITY' => '0.75',
29+
'CI_QUEUE_IDLE_EXIT_GRACE' => '5',
30+
)
31+
assert_equal 0.75, config.idle_exit_probability
32+
assert_equal 5.0, config.idle_exit_grace
33+
end
34+
2035
def test_heroku_ci_defaults
2136
config = Configuration.from_env(
2237
'HEROKU_TEST_RUN_ID' => 'YouAreAnAmazingPersonAndIBelieveYouCanDoIt',
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# frozen_string_literal: true
2+
require 'test_helper'
3+
4+
module CI::Queue::Redis
5+
class WorkerIdleExitTest < Minitest::Test
6+
REDIS_URL = 'redis://localhost:6379/0'
7+
8+
def test_worker_that_drew_requeue_duty_never_exits_on_idle
9+
worker = build_worker(idle_exit_probability: 0.0, idle_exit_grace: 0)
10+
assert worker.waits_for_requeues?
11+
12+
worker.idle_since = CI::Queue.time_now - 3600
13+
refute worker.idle_exit?
14+
end
15+
16+
def test_worker_without_requeue_duty_waits_out_the_grace_period
17+
worker = build_worker(idle_exit_probability: 1.0, idle_exit_grace: 30)
18+
refute worker.waits_for_requeues?
19+
20+
worker.idle_since = CI::Queue.time_now - 10
21+
refute worker.idle_exit?
22+
23+
worker.idle_since = CI::Queue.time_now - 31
24+
assert worker.idle_exit?
25+
end
26+
27+
def test_a_busy_worker_never_exits_on_idle
28+
worker = build_worker(idle_exit_probability: 1.0, idle_exit_grace: 0)
29+
assert_nil worker.idle_since
30+
refute worker.idle_exit?
31+
end
32+
33+
def test_slack_duration_is_unknown_until_a_test_finishes
34+
assert_nil build_worker.slack_duration
35+
end
36+
37+
def test_slack_duration_measures_from_the_last_finished_test
38+
worker = build_worker
39+
worker.instance_variable_set(:@last_test_finished_at, CI::Queue.time_now - 12)
40+
assert_in_delta 12, worker.slack_duration, 1
41+
end
42+
43+
private
44+
45+
def build_worker(**options)
46+
Worker.new(REDIS_URL, CI::Queue::Configuration.new(**options))
47+
end
48+
end
49+
end

0 commit comments

Comments
 (0)