From 2822526e7b996c90fb3bbd7c286c2777e5e37360 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 27 Oct 2016 12:34:09 +0100 Subject: [PATCH] Make retry_lock to not be infinite --- app/services/ci/process_pipeline_service.rb | 4 ++-- lib/gitlab/optimistic_locking.rb | 10 +++++++--- spec/lib/gitlab/optimistic_locking_spec.rb | 11 +++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 3b010269e0c..8face432d97 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -31,8 +31,8 @@ module Ci if HasStatus::COMPLETED_STATUSES.include?(current_status) created_builds_in_stage(index).select do |build| - Gitlab::OptimisticLocking.retry_lock(build) do |build| - process_build(build, current_status) + Gitlab::OptimisticLocking.retry_lock(build) do |subject| + process_build(subject, current_status) end end end diff --git a/lib/gitlab/optimistic_locking.rb b/lib/gitlab/optimistic_locking.rb index 17010d73c57..879d46446b3 100644 --- a/lib/gitlab/optimistic_locking.rb +++ b/lib/gitlab/optimistic_locking.rb @@ -1,12 +1,16 @@ module Gitlab - class OptimisticLocking - def self.retry_lock(subject, &block) + module OptimisticLocking + extend self + + def retry_lock(subject, retries = 100, &block) loop do begin - subject.transaction do + ActiveRecord::Base.transaction do return block.call(subject) end rescue ActiveRecord::StaleObjectError + retries -= 1 + raise unless retries >= 0 subject.reload end end diff --git a/spec/lib/gitlab/optimistic_locking_spec.rb b/spec/lib/gitlab/optimistic_locking_spec.rb index 75c78cf077a..498dc514c8c 100644 --- a/spec/lib/gitlab/optimistic_locking_spec.rb +++ b/spec/lib/gitlab/optimistic_locking_spec.rb @@ -24,5 +24,16 @@ describe Gitlab::OptimisticLocking, lib: true do subject.drop end end + + it 'raises exception when too many retries' do + expect(pipeline).to receive(:drop).twice.and_call_original + + expect do + described_class.retry_lock(pipeline, 1) do |subject| + subject.lock_version = 100 + subject.drop + end + end.to raise_error(ActiveRecord::StaleObjectError) + end end end