diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2017-11-27 12:21:26 +0000 |
---|---|---|
committer | Tiago <tiagonbotelho@hotmail.com> | 2017-11-28 12:53:50 +0000 |
commit | 97968f31f88188345d92e420a84826e58c984682 (patch) | |
tree | e1e297f9abf8e704459212881b7b4d219c20db2d | |
parent | 047dfd15f4988bdee90089f00c07cf5ae438d78b (diff) | |
download | gitlab-ce-97968f31f88188345d92e420a84826e58c984682.tar.gz |
Merge branch 'optimise-stuck-ci-jobs-worker' into 'master'
Optimise StuckCiJobsWorker
Closes gitlab-com/infrastructure#3260
See merge request gitlab-org/gitlab-ce!15527
(cherry picked from commit 420cdc6a5266df6de6767e3e3886e0019ae0ec6e)
a26e25ea Optimise StuckCiJobsWorker
d58bab4a Use not-ordered search
07c7ba1b Allow to drop jobs for deleted projects
6c1a4294 Fix stuck jobs tests
-rw-r--r-- | app/models/ci/build.rb | 1 | ||||
-rw-r--r-- | app/models/commit_status.rb | 16 | ||||
-rw-r--r-- | app/workers/stuck_ci_jobs_worker.rb | 14 | ||||
-rw-r--r-- | changelogs/unreleased/optimise-stuck-ci-jobs-worker.yml | 5 | ||||
-rw-r--r-- | spec/workers/stuck_ci_jobs_worker_spec.rb | 10 |
5 files changed, 32 insertions, 14 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 6ca46ae89c1..11a57e4b7cb 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -104,6 +104,7 @@ module Ci end before_transition any => [:failed] do |build| + next unless build.project next if build.retries_max.zero? if build.retries_count < build.retries_max diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 6b07dbdf3ea..ee21ed8e420 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -17,6 +17,7 @@ class CommitStatus < ActiveRecord::Base validates :name, presence: true, unless: :importing? alias_attribute :author, :user + alias_attribute :pipeline_id, :commit_id scope :failed_but_allowed, -> do where(allow_failure: true, status: [:failed, :canceled]) @@ -103,26 +104,29 @@ class CommitStatus < ActiveRecord::Base end after_transition do |commit_status, transition| + next unless commit_status.project next if transition.loopback? commit_status.run_after_commit do - if pipeline + if pipeline_id if complete? || manual? - PipelineProcessWorker.perform_async(pipeline.id) + PipelineProcessWorker.perform_async(pipeline_id) else - PipelineUpdateWorker.perform_async(pipeline.id) + PipelineUpdateWorker.perform_async(pipeline_id) end end - StageUpdateWorker.perform_async(commit_status.stage_id) - ExpireJobCacheWorker.perform_async(commit_status.id) + StageUpdateWorker.perform_async(stage_id) + ExpireJobCacheWorker.perform_async(id) end end after_transition any => :failed do |commit_status| + next unless commit_status.project + commit_status.run_after_commit do MergeRequests::AddTodoWhenBuildFailsService - .new(pipeline.project, nil).execute(self) + .new(project, nil).execute(self) end end end diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 269776a1f62..b0c739bbcb6 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -44,9 +44,17 @@ class StuckCiJobsWorker end def search(status, timeout) - builds = Ci::Build.where(status: status).where('ci_builds.updated_at < ?', timeout.ago) - builds.joins(:project).merge(Project.without_deleted).includes(:tags, :runner, project: :namespace).find_each(batch_size: 50).each do |build| - yield(build) + loop do + jobs = Ci::Build.where(status: status) + .where('ci_builds.updated_at < ?', timeout.ago) + .includes(:tags, :runner, project: :namespace) + .limit(100) + .to_a + break if jobs.empty? + + jobs.each do |job| + yield(job) + end end end diff --git a/changelogs/unreleased/optimise-stuck-ci-jobs-worker.yml b/changelogs/unreleased/optimise-stuck-ci-jobs-worker.yml new file mode 100644 index 00000000000..7f6adfb4fd8 --- /dev/null +++ b/changelogs/unreleased/optimise-stuck-ci-jobs-worker.yml @@ -0,0 +1,5 @@ +--- +title: Optimise StuckCiJobsWorker using cheap SQL query outside, and expensive inside +merge_request: +author: +type: performance diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index ac6f4fefb4e..bdc64c6785b 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -105,8 +105,8 @@ describe StuckCiJobsWorker do job.project.update(pending_delete: true) end - it 'does not drop job' do - expect_any_instance_of(Ci::Build).not_to receive(:drop) + it 'does drop job' do + expect_any_instance_of(Ci::Build).to receive(:drop).and_call_original worker.perform end end @@ -117,7 +117,7 @@ describe StuckCiJobsWorker do let(:worker2) { described_class.new } it 'is guard by exclusive lease when executed concurrently' do - expect(worker).to receive(:drop).at_least(:once) + expect(worker).to receive(:drop).at_least(:once).and_call_original expect(worker2).not_to receive(:drop) worker.perform allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false) @@ -125,8 +125,8 @@ describe StuckCiJobsWorker do end it 'can be executed in sequence' do - expect(worker).to receive(:drop).at_least(:once) - expect(worker2).to receive(:drop).at_least(:once) + expect(worker).to receive(:drop).at_least(:once).and_call_original + expect(worker2).to receive(:drop).at_least(:once).and_call_original worker.perform worker2.perform end |