Merge branch '332252-cancel-retry-nil' into 'master'

Fix retry-lock problem on pipeline cancel

See merge request gitlab-org/gitlab!63080

app/models/ci/pipeline.rb

@@ -577,11 +577,11 @@ module Ci
       canceled? && auto_canceled_by_id?
     end
 
-    def cancel_running(retries: nil)
+    def cancel_running(retries: 1)
       commit_status_relations = [:project, :pipeline]
       ci_build_relations = [:deployment, :taggings]
 
-      retry_optimistic_lock(cancelable_statuses, retries, name: 'ci_pipeline_cancel_running') do |cancelables|
+      retry_lock(cancelable_statuses, retries, name: 'ci_pipeline_cancel_running') do |cancelables|
         cancelables.find_in_batches do |batch|
           ActiveRecord::Associations::Preloader.new.preload(batch, commit_status_relations)
           ActiveRecord::Associations::Preloader.new.preload(batch.select { |job| job.is_a?(Ci::Build) }, ci_build_relations)
@@ -594,7 +594,7 @@ module Ci
       end
     end
 
-    def auto_cancel_running(pipeline, retries: nil)
+    def auto_cancel_running(pipeline, retries: 1)
       update(auto_canceled_by: pipeline)
 
       cancel_running(retries: retries) do |job|

spec/models/ci/pipeline_spec.rb

@@ -2768,6 +2768,41 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
         expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries)
       end
     end
+
+    context 'when the first try cannot get an exclusive lock' do
+      let(:retries) { 1 }
+
+      subject(:cancel_running) { pipeline.cancel_running(retries: retries) }
+
+      before do
+        build = create(:ci_build, :running, pipeline: pipeline)
+
+        allow(pipeline.cancelable_statuses).to receive(:find_in_batches).and_yield([build])
+
+        call_count = 0
+        allow(build).to receive(:cancel).and_wrap_original do |original, *args|
+          call_count >= retries ? raise(ActiveRecord::StaleObjectError) : original.call(*args)
+
+          call_count += 1
+        end
+      end
+
+      it 'retries again and cancels the build' do
+        cancel_running
+
+        expect(latest_status).to contain_exactly('canceled')
+      end
+
+      context 'when the retries parameter is 0' do
+        let(:retries) { 0 }
+
+        it 'raises error' do
+          expect do
+            cancel_running
+          end.to raise_error(ActiveRecord::StaleObjectError)
+        end
+      end
+    end
   end
 
   describe '#retry_failed' do