From 4da740279942800b02dcf675edae5c73510f58af Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sat, 30 Jun 2018 11:43:52 +0200 Subject: [PATCH 1/4] Deleting locks might cause problems for other jobs --- lib/sidekiq_unique_jobs/lock/base_lock.rb | 1 - .../server/middleware/until_and_while_executing_spec.rb | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/sidekiq_unique_jobs/lock/base_lock.rb b/lib/sidekiq_unique_jobs/lock/base_lock.rb index 7fe454e58..012a58b45 100644 --- a/lib/sidekiq_unique_jobs/lock/base_lock.rb +++ b/lib/sidekiq_unique_jobs/lock/base_lock.rb @@ -63,7 +63,6 @@ def prepare_item(item) def unlock_and_callback(callback) return notify_about_manual_unlock unless operative unlock - delete return notify_about_manual_unlock if locked? callback_safely(callback) diff --git a/spec/integration/sidekiq_unique_jobs/server/middleware/until_and_while_executing_spec.rb b/spec/integration/sidekiq_unique_jobs/server/middleware/until_and_while_executing_spec.rb index 8d349211c..843049280 100644 --- a/spec/integration/sidekiq_unique_jobs/server/middleware/until_and_while_executing_spec.rb +++ b/spec/integration/sidekiq_unique_jobs/server/middleware/until_and_while_executing_spec.rb @@ -60,6 +60,8 @@ version_key, available_run_key, available_key, + exists_run_key, + version_run_key, ]) end end From 8680b16a36297ce3975eca7537eccb5d34f3aa63 Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sat, 30 Jun 2018 11:44:16 +0200 Subject: [PATCH 2/4] Unused code --- lib/sidekiq_unique_jobs/unique_args.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/sidekiq_unique_jobs/unique_args.rb b/lib/sidekiq_unique_jobs/unique_args.rb index 7c5979f20..061c91174 100644 --- a/lib/sidekiq_unique_jobs/unique_args.rb +++ b/lib/sidekiq_unique_jobs/unique_args.rb @@ -6,8 +6,6 @@ module SidekiqUniqueJobs # This class exists to be testable and the entire api should be considered private class UniqueArgs - CLASS_NAME = 'SidekiqUniqueJobs::UniqueArgs' - include SidekiqUniqueJobs::Logging include SidekiqUniqueJobs::SidekiqWorkerMethods From b943ea198eb9f2cb922c8f181e18cf5d2d816e3c Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sat, 30 Jun 2018 11:44:28 +0200 Subject: [PATCH 3/4] Clean up before and after spec that uses redis --- spec/examples/my_unique_job_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/examples/my_unique_job_spec.rb b/spec/examples/my_unique_job_spec.rb index 442914286..d6d946e7f 100644 --- a/spec/examples/my_unique_job_spec.rb +++ b/spec/examples/my_unique_job_spec.rb @@ -19,14 +19,18 @@ let(:args) { %w[one two] } end - describe 'client middleware' do + describe 'client middleware', redis: :redis do context 'when job is delayed' do before { described_class.perform_in(3600, 1, 2) } it 'rejects new scheduled jobs' do expect(1).to be_enqueued_in('customqueue') described_class.perform_in(3600, 1, 2) + described_class.perform_in(3600, 1, 2) + described_class.perform_in(3600, 1, 2) expect(1).to be_enqueued_in('customqueue') + expect(1).to be_enqueued_in('schedule') + expect(zcount('schedule')).to eq(1) expect(1).to be_scheduled_at(Time.now.to_f + 2 * 3600) end From dddd7d3055f37561145a4e2f80afb5568bf6d5a0 Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sat, 30 Jun 2018 11:47:29 +0200 Subject: [PATCH 4/4] Use zcard since we want the total number in queue --- spec/examples/my_unique_job_spec.rb | 2 +- .../sidekiq_unique_jobs/client/middleware_spec.rb | 4 ++++ spec/support/sidekiq_helpers.rb | 8 ++++---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/examples/my_unique_job_spec.rb b/spec/examples/my_unique_job_spec.rb index d6d946e7f..d0e13022b 100644 --- a/spec/examples/my_unique_job_spec.rb +++ b/spec/examples/my_unique_job_spec.rb @@ -30,7 +30,7 @@ described_class.perform_in(3600, 1, 2) expect(1).to be_enqueued_in('customqueue') expect(1).to be_enqueued_in('schedule') - expect(zcount('schedule')).to eq(1) + expect(zcard('schedule')).to eq(1) expect(1).to be_scheduled_at(Time.now.to_f + 2 * 3600) end diff --git a/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb b/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb index 274c38499..59b03f303 100644 --- a/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb +++ b/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb @@ -21,6 +21,10 @@ it 'rejects nested subsequent jobs with the same arguments' do expect(SimpleWorker.perform_async(1)).not_to eq(nil) expect(SimpleWorker.perform_async(1)).to eq(nil) + expect(SimpleWorker.perform_in(60, 1)).to eq(nil) + expect(SimpleWorker.perform_in(60, 1)).to eq(nil) + expect(SimpleWorker.perform_in(60, 1)).to eq(nil) + expect(zcard('schedule')).to eq(0) expect(SpawnSimpleWorker.perform_async(1)).not_to eq(nil) expect(queue_count('default')).to eq(1) diff --git a/spec/support/sidekiq_helpers.rb b/spec/support/sidekiq_helpers.rb index 9507e02a5..6d3431134 100644 --- a/spec/support/sidekiq_helpers.rb +++ b/spec/support/sidekiq_helpers.rb @@ -11,8 +11,8 @@ def zcard(queue) redis { |conn| conn.zcard(queue) } end - def zcount(queue, time) - redis { |conn| conn.zcount(queue, -1, time) } + def zcount(queue, min = '-inf', max = '+inf') + redis { |conn| conn.zcount(queue, min, max) } end def hexists(hash, key) @@ -39,8 +39,8 @@ def schedule_count zcard('schedule') end - def schedule_count_at(time = Time.now.to_f + 2 * 60) - zcount('schedule', time) + def schedule_count_at(max = Time.now.to_f + 2 * 60) + zcount('schedule', '-inf', max) end def queue_count(queue)