Merge branch 'use-exclusive-lease-module' into 'master'

Use ExclusiveLeaseGuard module in StuckBuilds workers

See merge request gitlab-org/gitlab!71755

app/workers/ci/stuck_builds/drop_running_worker.rb

@@ -4,6 +4,7 @@ module Ci
   module StuckBuilds
     class DropRunningWorker
       include ApplicationWorker
+      include ExclusiveLeaseGuard
 
       idempotent!
 
@@ -17,26 +18,16 @@ module Ci
 
       feature_category :continuous_integration
 
-      EXCLUSIVE_LEASE_KEY = 'ci_stuck_builds_drop_running_worker_lease'
-
       def perform
-        return unless try_obtain_lease
-
-        begin
+        try_obtain_lease do
           Ci::StuckBuilds::DropRunningService.new.execute
-        ensure
-          remove_lease
         end
       end
 
       private
 
-      def try_obtain_lease
-        @uuid = Gitlab::ExclusiveLease.new(EXCLUSIVE_LEASE_KEY, timeout: 30.minutes).try_obtain
-      end
-
-      def remove_lease
-        Gitlab::ExclusiveLease.cancel(EXCLUSIVE_LEASE_KEY, @uuid)
+      def lease_timeout
+        30.minutes
       end
     end
   end

app/workers/stuck_ci_jobs_worker.rb

@@ -2,6 +2,7 @@
 
 class StuckCiJobsWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
+  include ExclusiveLeaseGuard
 
   # rubocop:disable Scalability/CronWorkerContext
   # This is an instance-wide cleanup query, so there's no meaningful
@@ -14,28 +15,18 @@ class StuckCiJobsWorker # rubocop:disable Scalability/IdempotentWorker
   feature_category :continuous_integration
   worker_resource_boundary :cpu
 
-  EXCLUSIVE_LEASE_KEY = 'stuck_ci_builds_worker_lease'
-
   def perform
     Ci::StuckBuilds::DropRunningWorker.perform_in(20.minutes)
     Ci::StuckBuilds::DropScheduledWorker.perform_in(40.minutes)
 
-    return unless try_obtain_lease
-
-    begin
+    try_obtain_lease do
       Ci::StuckBuilds::DropService.new.execute
-    ensure
-      remove_lease
     end
   end
 
   private
 
-  def try_obtain_lease
-    @uuid = Gitlab::ExclusiveLease.new(EXCLUSIVE_LEASE_KEY, timeout: 30.minutes).try_obtain
-  end
-
-  def remove_lease
-    Gitlab::ExclusiveLease.cancel(EXCLUSIVE_LEASE_KEY, @uuid)
+  def lease_timeout
+    30.minutes
   end
 end

spec/workers/ci/stuck_builds/drop_running_worker_spec.rb

@@ -5,66 +5,24 @@ require 'spec_helper'
 RSpec.describe Ci::StuckBuilds::DropRunningWorker do
   include ExclusiveLeaseHelpers
 
-  let(:worker_lease_key)  { Ci::StuckBuilds::DropRunningWorker::EXCLUSIVE_LEASE_KEY }
-  let(:worker_lease_uuid) { SecureRandom.uuid }
-  let(:worker2)           { described_class.new }
-
-  subject(:worker) { described_class.new }
-
-  before do
-    stub_exclusive_lease(worker_lease_key, worker_lease_uuid)
-  end
+  let(:worker)     { described_class.new }
+  let(:lease_uuid) { SecureRandom.uuid }
 
   describe '#perform' do
+    subject { worker.perform }
+
     it_behaves_like 'an idempotent worker'
 
     it 'executes an instance of Ci::StuckBuilds::DropRunningService' do
+      expect_to_obtain_exclusive_lease(worker.lease_key, lease_uuid)
+
       expect_next_instance_of(Ci::StuckBuilds::DropRunningService) do |service|
         expect(service).to receive(:execute).exactly(:once)
       end
 
-      worker.perform
-    end
-
-    context 'with an exclusive lease' do
-      it 'does not execute concurrently' do
-        expect(worker).to receive(:remove_lease).exactly(:once)
-        expect(worker2).not_to receive(:remove_lease)
-
-        worker.perform
-
-        stub_exclusive_lease_taken(worker_lease_key)
-
-        worker2.perform
-      end
-
-      it 'can execute in sequence' do
-        expect(worker).to receive(:remove_lease).at_least(:once)
-        expect(worker2).to receive(:remove_lease).at_least(:once)
-
-        worker.perform
-        worker2.perform
-      end
-
-      it 'cancels exclusive leases after worker perform' do
-        expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
+      expect_to_cancel_exclusive_lease(worker.lease_key, lease_uuid)
 
-        worker.perform
-      end
-
-      context 'when the DropRunningService fails' do
-        it 'ensures cancellation of the exclusive lease' do
-          expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
-
-          allow_next_instance_of(Ci::StuckBuilds::DropRunningService) do |service|
-            expect(service).to receive(:execute) do
-              raise 'The query timed out'
-            end
-          end
-
-          expect { worker.perform }.to raise_error(/The query timed out/)
-        end
-      end
+      subject
     end
   end
 end

spec/workers/stuck_ci_jobs_worker_spec.rb

@@ -5,76 +5,34 @@ require 'spec_helper'
 RSpec.describe StuckCiJobsWorker do
   include ExclusiveLeaseHelpers
 
-  let(:worker_lease_key)  { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY }
-  let(:worker_lease_uuid) { SecureRandom.uuid }
-  let(:worker2)           { described_class.new }
-
-  subject(:worker) { described_class.new }
-
-  before do
-    stub_exclusive_lease(worker_lease_key, worker_lease_uuid)
-  end
+  let(:worker)     { described_class.new }
+  let(:lease_uuid) { SecureRandom.uuid }
 
   describe '#perform' do
+    subject { worker.perform }
+
     it 'enqueues a Ci::StuckBuilds::DropRunningWorker job' do
       expect(Ci::StuckBuilds::DropRunningWorker).to receive(:perform_in).with(20.minutes).exactly(:once)
 
-      worker.perform
+      subject
     end
 
     it 'enqueues a Ci::StuckBuilds::DropScheduledWorker job' do
       expect(Ci::StuckBuilds::DropScheduledWorker).to receive(:perform_in).with(40.minutes).exactly(:once)
 
-      worker.perform
+      subject
     end
 
     it 'executes an instance of Ci::StuckBuilds::DropService' do
+      expect_to_obtain_exclusive_lease(worker.lease_key, lease_uuid)
+
       expect_next_instance_of(Ci::StuckBuilds::DropService) do |service|
         expect(service).to receive(:execute).exactly(:once)
       end
 
-      worker.perform
-    end
-
-    context 'with an exclusive lease' do
-      it 'does not execute concurrently' do
-        expect(worker).to receive(:remove_lease).exactly(:once)
-        expect(worker2).not_to receive(:remove_lease)
-
-        worker.perform
-
-        stub_exclusive_lease_taken(worker_lease_key)
-
-        worker2.perform
-      end
-
-      it 'can execute in sequence' do
-        expect(worker).to receive(:remove_lease).at_least(:once)
-        expect(worker2).to receive(:remove_lease).at_least(:once)
-
-        worker.perform
-        worker2.perform
-      end
-
-      it 'cancels exclusive leases after worker perform' do
-        expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
+      expect_to_cancel_exclusive_lease(worker.lease_key, lease_uuid)
 
-        worker.perform
-      end
-
-      context 'when the DropService fails' do
-        it 'ensures cancellation of the exclusive lease' do
-          expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
-
-          allow_next_instance_of(Ci::StuckBuilds::DropService) do |service|
-            expect(service).to receive(:execute) do
-              raise 'The query timed out'
-            end
-          end
-
-          expect { worker.perform }.to raise_error(/The query timed out/)
-        end
-      end
+      subject
     end
   end
 end