Merge branch '10io-sidekiq-ensure-removing-deduplication-key-if-until-executed' into 'master'

Delete the duplicate job even in case of errors

See merge request gitlab-org/gitlab!87700

lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb

@@ -10,6 +10,8 @@ module Strategies
         class UntilExecuted < DeduplicatesWhenScheduling
           override :perform
           def perform(job)
+            job_deleted = false
+
             super
 
             yield
@@ -17,7 +19,10 @@ def perform(job)
             should_reschedule = duplicate_job.should_reschedule?
             # Deleting before rescheduling to make sure we don't deduplicate again.
             duplicate_job.delete!
+            job_deleted = true
             duplicate_job.reschedule if should_reschedule
+          ensure
+            duplicate_job.delete! unless job_deleted
           end
         end
       end

spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb

@@ -23,8 +23,15 @@
         end
       end
 
+      it 'deletes the lock even if an error occurs' do
+        expect(fake_duplicate_job).not_to receive(:scheduled?)
+        expect(fake_duplicate_job).to receive(:delete!).once
+
+        perform_strategy_with_error
+      end
+
       it 'does not reschedule the job even if deduplication happened' do
-        expect(fake_duplicate_job).to receive(:delete!)
+        expect(fake_duplicate_job).to receive(:delete!).once
         expect(fake_duplicate_job).not_to receive(:reschedule)
 
         strategy.perform({}) do
@@ -33,16 +40,33 @@
       end
 
       context 'when job is reschedulable' do
-        it 'reschedules the job if deduplication happened' do
+        before do
           allow(fake_duplicate_job).to receive(:should_reschedule?) { true }
+        end
 
-          expect(fake_duplicate_job).to receive(:delete!)
+        it 'reschedules the job if deduplication happened' do
+          expect(fake_duplicate_job).to receive(:delete!).once
           expect(fake_duplicate_job).to receive(:reschedule).once
 
           strategy.perform({}) do
             proc.call
           end
         end
+
+        it 'does not reschedule the job if an error occurs' do
+          expect(fake_duplicate_job).to receive(:delete!).once
+          expect(fake_duplicate_job).not_to receive(:reschedule)
+
+          perform_strategy_with_error
+        end
+      end
+
+      def perform_strategy_with_error
+        expect do
+          strategy.perform({}) do
+            raise 'expected error'
+          end
+        end.to raise_error(RuntimeError, 'expected error')
       end
     end
   end