From 3cb78cac2b862207fbbd9ade968b0f4c582c0752 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 20 Oct 2016 09:33:44 +0200 Subject: [PATCH 1/5] Use synchronous pipeline processing --- app/models/commit_status.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 7b554be4f9a5..48e7c74de110 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -91,9 +91,9 @@ class CommitStatus < ActiveRecord::Base commit_status.run_after_commit do pipeline.try do |pipeline| if complete? - PipelineProcessWorker.perform_async(pipeline.id) + PipelineProcessWorker.perform(pipeline.id) else - PipelineUpdateWorker.perform_async(pipeline.id) + PipelineUpdateWorker.perform(pipeline.id) end end end -- GitLab From 10e7acc10907a847670c5ea37ce8aebfcd5349f5 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 20 Oct 2016 18:45:16 +0200 Subject: [PATCH 2/5] Reduce number of PipelineUpdateWorkers and PipelineProcessWorkers by verifying if it makes sense to do update --- app/models/commit_status.rb | 33 ++++++++++++++++----- app/services/ci/process_pipeline_service.rb | 27 +++++++---------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 48e7c74de110..08f4735c2a20 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -85,17 +85,34 @@ class CommitStatus < ActiveRecord::Base commit_status.update_attributes finished_at: Time.now end - after_transition do |commit_status, transition| + after_transition any => [:pending] do |commit_status, transition| next if transition.loopback? + next unless commit_status.pipeline commit_status.run_after_commit do - pipeline.try do |pipeline| - if complete? - PipelineProcessWorker.perform(pipeline.id) - else - PipelineUpdateWorker.perform(pipeline.id) - end - end + next if pipeline.reload.active? + + PipelineUpdateWorker.perform_async(pipeline.id) + end + end + + after_transition any => [:running] do |commit_status, transition| + next if transition.loopback? + next unless commit_status.pipeline + + commit_status.run_after_commit do + next if pipeline.reload.running? + + PipelineUpdateWorker.perform_async(pipeline.id) + end + end + + after_transition any => HasStatus::COMPLETED_STATUSES do |commit_status, transition| + next if transition.loopback? + next unless commit_status.pipeline + + commit_status.run_after_commit do + PipelineProcessWorker.perform_async(pipeline.id) end end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index d3dd30b2588f..d5c707aca79e 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -10,17 +10,12 @@ def execute(pipeline) create_builds! end - @pipeline.with_lock do - new_builds = - stage_indexes_of_created_builds.map do |index| - process_stage(index) - end - - @pipeline.update_status + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end - # Return a flag if a when builds got enqueued - new_builds.flatten.any? - end + @pipeline.update_status if new_builds.flatten.any? end private @@ -40,12 +35,12 @@ def process_stage(index) end def process_build(build, current_status) - if valid_statuses_for_when(build.when).include?(current_status) - build.enqueue - true - else - build.skip - false + build.with_lock do + if valid_statuses_for_when(build.when).include?(current_status) + build.enqueue + else + build.skip + end end end -- GitLab From 2ad7b245391eb261b3d9f0729207e4bb671d3429 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 20 Oct 2016 18:50:08 +0200 Subject: [PATCH 3/5] Reduce number of locks and queries --- app/services/ci/process_pipeline_service.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index d5c707aca79e..e7a422f9d282 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -16,6 +16,8 @@ def execute(pipeline) end @pipeline.update_status if new_builds.flatten.any? + + new_builds.flatten.any? end private @@ -27,8 +29,8 @@ def create_builds! def process_stage(index) current_status = status_for_prior_stages(index) - created_builds_in_stage(index).select do |build| - if HasStatus::COMPLETED_STATUSES.include?(current_status) + if HasStatus::COMPLETED_STATUSES.include?(current_status) + created_builds_in_stage(index).select do |build| process_build(build, current_status) end end -- GitLab From d6b090d7582736694683f64c413424fa14812bd6 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 20 Oct 2016 20:47:50 +0200 Subject: [PATCH 4/5] Use symbols for statuses --- app/models/commit_status.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 08f4735c2a20..e6c6a28648fb 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -107,7 +107,7 @@ class CommitStatus < ActiveRecord::Base end end - after_transition any => HasStatus::COMPLETED_STATUSES do |commit_status, transition| + after_transition any => [:success, :failed, :canceled] do |commit_status, transition| next if transition.loopback? next unless commit_status.pipeline -- GitLab From 4f6bd53ce4558b3b121f1b6602c201919c8542ec Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 21 Oct 2016 10:02:12 +0200 Subject: [PATCH 5/5] Execute lock only when it makes sense --- app/models/ci/pipeline.rb | 24 ++++++++++++++++----- app/services/ci/process_pipeline_service.rb | 2 +- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e75fe6c222bf..26f1c00286cc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -30,23 +30,23 @@ class Pipeline < ActiveRecord::Base end event :run do - transition any => :running + transition any - [:running] => :running end event :skip do - transition any => :skipped + transition any - [:skipped] => :skipped end event :drop do - transition any => :failed + transition any - [:failed] => :failed end event :succeed do - transition any => :success + transition any - [:success] => :success end event :cancel do - transition any => :canceled + transition any - [:canceled] => :canceled end # IMPORTANT @@ -263,6 +263,20 @@ def process! end def update_status + reload + + can_update = + case latest_builds_status + when 'pending' then can_enqueue? + when 'running' then can_run? + when 'success' then can_succeed? + when 'failed' then can_drop? + when 'canceled' then can_cancel? + when 'skipped' then can_skip? + end + + return unless can_update + with_lock do case latest_builds_status when 'pending' then enqueue diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index e7a422f9d282..3b255dcc803d 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -15,7 +15,7 @@ def execute(pipeline) process_stage(index) end - @pipeline.update_status if new_builds.flatten.any? + @pipeline.update_status new_builds.flatten.any? end -- GitLab