summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-04-28 10:27:08 +0700
committerShinya Maeda <shinya@gitlab.com>2019-04-28 12:59:23 +0700
commit28f785404a6659d61c69ee4bfdaca915652d1759 (patch)
tree0ba0c26fdc6e685538e6a6d6ab6f2ffbf174f749
parentf04f6909cebac79704081bc4787099c613a70cf7 (diff)
downloadgitlab-ce-28f785404a6659d61c69ee4bfdaca915652d1759.tar.gz
Prevent concurrent execution of PipelineScheduleWorker
Currently, PipelineScheduleWorker is fired in a short period on our production server. We can stop this behavior by locking the execution thread with in_lock method.
-rw-r--r--app/workers/pipeline_schedule_worker.rb28
-rw-r--r--changelogs/unreleased/lock-pipeline-schedule-worker.yml5
-rw-r--r--spec/workers/pipeline_schedule_worker_spec.rb12
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)