diff options
author | Stan Hu <stanhu@gmail.com> | 2019-04-29 17:18:34 +0000 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-04-29 17:18:34 +0000 |
commit | ca8e5aded8e5e63b97a511ca7e50cc981ac7c4dd (patch) | |
tree | c2997a437de5b7ee0ad64adaf932cc27f69619b5 | |
parent | bb733375d1f7868cb713e4fd92c6f52956650b92 (diff) | |
parent | 28f785404a6659d61c69ee4bfdaca915652d1759 (diff) | |
download | gitlab-ce-ca8e5aded8e5e63b97a511ca7e50cc981ac7c4dd.tar.gz |
Merge branch 'lock-pipeline-schedule-worker' into 'master'
Prevent concurrent execution of PipelineScheduleWorker
Closes gitlab-com/gl-infra/production#805
See merge request gitlab-org/gitlab-ce!27781
-rw-r--r-- | app/workers/pipeline_schedule_worker.rb | 28 | ||||
-rw-r--r-- | changelogs/unreleased/lock-pipeline-schedule-worker.yml | 5 | ||||
-rw-r--r-- | spec/workers/pipeline_schedule_worker_spec.rb | 12 |
3 files changed, 34 insertions, 11 deletions
diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index 02a69ea3b54..8a9ee7808e4 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -3,20 +3,26 @@ class PipelineScheduleWorker include ApplicationWorker include CronjobQueue + include ::Gitlab::ExclusiveLeaseHelpers + + EXCLUSIVE_LOCK_KEY = 'pipeline_schedules:run:lock' + LOCK_TIMEOUT = 50.minutes # rubocop: disable CodeReuse/ActiveRecord def perform - Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) - .preload(:owner, :project).find_each do |schedule| - - Ci::CreatePipelineService.new(schedule.project, - schedule.owner, - ref: schedule.ref) - .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule) - rescue => e - error(schedule, e) - ensure - schedule.schedule_next_run! + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) + .preload(:owner, :project).find_each do |schedule| + + schedule.schedule_next_run! + + Ci::CreatePipelineService.new(schedule.project, + schedule.owner, + ref: schedule.ref) + .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule) + rescue => e + error(schedule, e) + end end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/changelogs/unreleased/lock-pipeline-schedule-worker.yml b/changelogs/unreleased/lock-pipeline-schedule-worker.yml new file mode 100644 index 00000000000..1b889f01620 --- /dev/null +++ b/changelogs/unreleased/lock-pipeline-schedule-worker.yml @@ -0,0 +1,5 @@ +--- +title: Prevent concurrent execution of PipelineScheduleWorker +merge_request: 27781 +author: +type: performance diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index f23910d23be..8c604b13297 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe PipelineScheduleWorker do + include ExclusiveLeaseHelpers + subject { described_class.new.perform } set(:project) { create(:project, :repository) } @@ -39,6 +41,16 @@ describe PipelineScheduleWorker do it_behaves_like 'successful scheduling' + context 'when exclusive lease has already been taken by the other instance' do + before do + stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY, timeout: described_class::LOCK_TIMEOUT) + end + + it 'raises an error and does not start creating pipelines' do + expect { subject }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + end + end + context 'when the latest commit contains [ci skip]' do before do allow_any_instance_of(Ci::Pipeline) |