From e0cc2db08ff23244bb7eebe2bc7e6c51c2c85525 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Sep 2016 15:03:51 +0200 Subject: [PATCH 1/2] Make pipeline processing asynchronous --- app/models/ci/build.rb | 2 +- app/models/ci/pipeline.rb | 26 +++++++++++++------ app/models/commit_status.rb | 17 ++++-------- .../add_todo_when_build_fails_service.rb | 8 +++--- app/services/merge_requests/base_service.rb | 15 +++++------ .../merge_when_build_succeeds_service.rb | 7 ++--- app/workers/process_pipeline_worker.rb | 17 ++++++++++++ 7 files changed, 55 insertions(+), 37 deletions(-) create mode 100644 app/workers/process_pipeline_worker.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d5724af4cce9..ca4a3e0ca79f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -61,7 +61,7 @@ def retry(build, user = nil) environment: build.environment, status_event: 'enqueue' ) - MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build) + MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build.pipeline) build.pipeline.mark_as_processable_after_stage(build.stage_idx) new_build end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0b1df9f42942..44be48d5db9e 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -62,6 +62,14 @@ class Pipeline < ActiveRecord::Base after_transition do |pipeline, transition| pipeline.execute_hooks unless transition.loopback? end + + after_transition [:created, :pending, :running] => :success do |pipeline| + MergeRequests::MergeWhenBuildSucceedsService.new(pipeline.project, nil).trigger(pipeline) + end + + after_transition any => :failed do |pipeline| + MergeRequests::AddTodoWhenBuildFailsService.new(pipeline.project, nil).execute(pipeline) + end end # ref can't be HEAD or SHA, can only be branch/tag name @@ -240,14 +248,16 @@ def process! Ci::ProcessPipelineService.new(project, user).execute(self) end - def build_updated - case latest_builds_status - when 'pending' then enqueue - when 'running' then run - when 'success' then succeed - when 'failed' then drop - when 'canceled' then cancel - when 'skipped' then skip + def update_status + with_lock do + case latest_builds_status + when 'pending' then enqueue + when 'running' then run + when 'success' then succeed + when 'failed' then drop + when 'canceled' then cancel + when 'skipped' then skip + end end end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index c85561291c87..e2009b9194b7 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -70,21 +70,14 @@ class CommitStatus < ActiveRecord::Base end after_transition do |commit_status, transition| - commit_status.pipeline.try(:build_updated) unless transition.loopback? - end + if commit_status.pipeline && !transition.loopback? + ProcessPipelineWorker.perform_async( + commit_status.pipeline.id, + process: HasStatus.COMPLETED_STATUSES.include?(commit_status.status)) + end - after_transition any => [:success, :failed, :canceled] do |commit_status| - commit_status.pipeline.try(:process!) true end - - after_transition [:created, :pending, :running] => :success do |commit_status| - MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status) - end - - after_transition any => :failed do |commit_status| - MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status) - end end delegate :sha, :short_sha, to: :pipeline diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb index 566049525cb7..45c737898861 100644 --- a/app/services/merge_requests/add_todo_when_build_fails_service.rb +++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb @@ -1,15 +1,15 @@ module MergeRequests class AddTodoWhenBuildFailsService < MergeRequests::BaseService # Adds a todo to the parent merge_request when a CI build fails - def execute(commit_status) - each_merge_request(commit_status) do |merge_request| + def execute(pipeline) + each_merge_request(pipeline) do |merge_request| todo_service.merge_request_build_failed(merge_request) end end # Closes any pending build failed todos for the parent MRs when a build is retried - def close(commit_status) - each_merge_request(commit_status) do |merge_request| + def close(pipeline) + each_merge_request(pipeline) do |merge_request| todo_service.merge_request_build_retried(merge_request) end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index ba424b09463a..0243124b4836 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -41,11 +41,11 @@ def filter_params super(:merge_request) end - def merge_request_from(commit_status) - branches = commit_status.ref + def merge_request_from(pipeline) + branches = pipeline.ref # This is for ref-less builds - branches ||= @project.repository.branch_names_contains(commit_status.sha) + branches ||= @project.repository.branch_names_contains(pipeline.sha) return [] if branches.blank? @@ -55,14 +55,11 @@ def merge_request_from(commit_status) merge_requests.uniq.select(&:source_project) end - def each_merge_request(commit_status) + def each_merge_request(pipeline) merge_request_from(commit_status).each do |merge_request| - pipeline = merge_request.pipeline + next unless pipeline == merge_request.pipeline - next unless pipeline - next unless pipeline.sha == commit_status.sha - - yield merge_request, pipeline + yield merge_request end end end diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb index 4ad5fb083114..60d6085f9eb6 100644 --- a/app/services/merge_requests/merge_when_build_succeeds_service.rb +++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb @@ -19,11 +19,12 @@ def execute(merge_request) end # Triggers the automatic merge of merge_request once the build succeeds - def trigger(commit_status) - each_merge_request(commit_status) do |merge_request, pipeline| + def trigger(pipeline) + return unless pipeline.success? + + each_merge_request(pipeline) do |merge_request| next unless merge_request.merge_when_build_succeeds? next unless merge_request.mergeable? - next unless pipeline.success? MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params) end diff --git a/app/workers/process_pipeline_worker.rb b/app/workers/process_pipeline_worker.rb new file mode 100644 index 000000000000..fb59a1efb7a2 --- /dev/null +++ b/app/workers/process_pipeline_worker.rb @@ -0,0 +1,17 @@ +class ProcessPipelineWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(pipeline_id, params) + begin + pipeline = Ci::Pipeline.find(pipeline_id) + rescue ActiveRecord::RecordNotFound + return + end + + pipeline.process! if params[:process] + + pipeline.update_status + end +end -- GitLab From 8af809104f371f7243e551a39fc4b9dd15dcd08a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Sep 2016 17:22:46 +0200 Subject: [PATCH 2/2] Fix async processing --- app/models/commit_status.rb | 2 +- app/services/merge_requests/base_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index e2009b9194b7..73bcf6a37f01 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -73,7 +73,7 @@ class CommitStatus < ActiveRecord::Base if commit_status.pipeline && !transition.loopback? ProcessPipelineWorker.perform_async( commit_status.pipeline.id, - process: HasStatus.COMPLETED_STATUSES.include?(commit_status.status)) + process: COMPLETED_STATUSES.include?(commit_status.status)) end true diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 0243124b4836..9b30f22c21f9 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -56,7 +56,7 @@ def merge_request_from(pipeline) end def each_merge_request(pipeline) - merge_request_from(commit_status).each do |merge_request| + merge_request_from(pipeline).each do |merge_request| next unless pipeline == merge_request.pipeline yield merge_request -- GitLab