From 9555540968fac2cea458a571240f0ba7ac7216f7 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Tue, 30 Nov 2021 11:04:40 +0100 Subject: [PATCH] Fix edge case in ruby reaper Closes #653 The problem was that the reaper returns nil instead of the memo. Many thanks to @francesmcmullin for the wonderfully detailed report and investigation. Now we have a regression test for this behavior. --- .../orphans/ruby_reaper.rb | 2 +- .../orphans/ruby_reaper_spec.rb | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb index 0c816cfb7..0607b2abe 100644 --- a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb +++ b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb @@ -59,7 +59,7 @@ def orphans next if belongs_to_job?(digest) memo << digest - break if memo.size >= reaper_count + break memo if memo.size >= reaper_count end end diff --git a/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb b/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb index 45d20dbef..94a209992 100644 --- a/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb +++ b/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb @@ -32,6 +32,28 @@ describe "#orphans" do subject(:orphans) { service.orphans } + context "when reaping more jobs than reaper_count" do + let(:digest_one) { "uniquejobs:digest1" } + let(:digest_two) { "uniquejobs:digest2" } + let(:digest_three) { "uniquejobs:digest3" } + let(:job_id_one) { "jobid1" } + let(:job_id_two) { "jobid2" } + let(:job_id_three) { "jobid3" } + + before do + SidekiqUniqueJobs::Lock.create(digest_one, job_id_one) + SidekiqUniqueJobs::Lock.create(digest_two, job_id_two) + SidekiqUniqueJobs::Lock.create(digest_three, job_id_three) + end + + it 'returns the first digest' do + SidekiqUniqueJobs.use_config(reaper_count: 1) do + expect(orphans.size).to eq(1) + expect([digest_one, digest_two, digest_three]).to include(orphans.first) + end + end + end + context "when scheduled" do let(:item) { raw_item.merge("at" => Time.now.to_f + 3_600) }