Skip to content

Commit 7916139

Browse files
authored
Merge pull request #191 from Shopify/cbruckmayer/fix-retry
Fix created_at race condition
2 parents 1a429d1 + c7032bb commit 7916139

File tree

6 files changed

+23
-5
lines changed

6 files changed

+23
-5
lines changed

ruby/lib/ci/queue/redis/base.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,18 @@ def exhausted?
2222
end
2323

2424
def expired?
25-
if (created_at = redis.get(key('master-created-at')))
25+
if (created_at = redis.get(key('created-at')))
2626
(created_at.to_f + config.redis_ttl + TEN_MINUTES) < Time.now.to_f
2727
else
2828
# if there is no created at set anymore we assume queue is expired
2929
true
3030
end
3131
end
3232

33+
def created_at=(timestamp)
34+
redis.setnx(key('created-at'), timestamp)
35+
end
36+
3337
def size
3438
redis.multi do |transaction|
3539
transaction.llen(key('queue'))
@@ -71,6 +75,10 @@ def queue_initialized?
7175
end
7276
end
7377

78+
def queue_initializing?
79+
master_status == 'setup'
80+
end
81+
7482
def increment_test_failed
7583
redis.incr(key('test_failed_count'))
7684
end

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,10 @@ def push(tests)
202202
transaction.lpush(key('queue'), tests) unless tests.empty?
203203
transaction.set(key('total'), @total)
204204
transaction.set(key('master-status'), 'ready')
205-
transaction.set(key('master-created-at'), Time.now.to_f)
206205

207206
transaction.expire(key('queue'), config.redis_ttl)
208207
transaction.expire(key('total'), config.redis_ttl)
209208
transaction.expire(key('master-status'), config.redis_ttl)
210-
transaction.expire(key('master-created-at'), config.redis_ttl)
211209
end
212210
end
213211
register

ruby/lib/ci/queue/static.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ def from_uri(uri, config)
1111
end
1212
end
1313

14+
TEN_MINUTES = 60 * 10
15+
1416
attr_reader :progress, :total
1517

1618
def initialize(tests, config)
@@ -37,6 +39,14 @@ def populate(tests, random: nil)
3739
self
3840
end
3941

42+
def created_at=(timestamp)
43+
@created_at ||= timestamp
44+
end
45+
46+
def expired?
47+
(@created_at.to_f TEN_MINUTES) < Time.now.to_f
48+
end
49+
4050
def populated?
4151
!!defined?(@index)
4252
end

ruby/lib/ci/queue/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
module CI
44
module Queue
5-
VERSION = '0.24.0'
5+
VERSION = '0.24.1'
66
DEV_SCRIPTS_ROOT = ::File.expand_path('../../../../../redis', __FILE__)
77
RELEASE_SCRIPTS_ROOT = ::File.expand_path('../redis', __FILE__)
88
end

ruby/lib/minitest/queue/runner.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ def run_command
6161
end
6262
end
6363

64+
queue.rescue_connection_errors { queue.created_at = Time.now.to_f }
65+
6466
set_load_path
6567
Minitest.queue = queue
6668
reporters = [

ruby/test/integration/minitest_redis_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ def test_retry_fails_when_test_run_is_expired
238238
assert_equal 'Ran 100 tests, 100 assertions, 0 failures, 0 errors, 0 skips, 0 requeues in X.XXs', output
239239

240240
one_day = 60 * 60 * 24
241-
key = ['build', "1", "master-created-at"].join(':')
241+
key = ['build', "1", "created-at"].join(':')
242242
@redis.set(key, Time.now - one_day)
243243

244244
out, err = capture_subprocess_io do

0 commit comments

Comments
 (0)