Merge branch 'feature/improve-async-pipeline-processing' into 'master'

Improve asynchronous pipeline processing

## What does this MR do?

This MR improves asynchronous processing of pipeline.

## Why was this MR needed?

It eliminates some race conditions and improves performance.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing

## What are the relevant issue / merge request numbers?

Related merge request:  !6410  
Extracted from !6411

See merge request !6650

10 files changed, 80 insertions, 12 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 97df74b0cfe..2cf9892edc5 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -251,9 +251,8 @@ module Ci
       Ci::ProcessPipelineService.new(project, user).execute(self)
     end
 
-    def build_updated
+    def update_status
       with_lock do
-        reload
         case latest_builds_status
         when 'pending' then enqueue
         when 'running' then run
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index ee3396abe04..9fa8d17e74e 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -84,13 +84,18 @@ class CommitStatus < ActiveRecord::Base
       commit_status.update_attributes finished_at: Time.now
     end
 
-    after_transition any => [:success, :failed, :canceled] do |commit_status|
-      commit_status.pipeline.try(:process!)
-      true
-    end
-
     after_transition do |commit_status, transition|
-      commit_status.pipeline.try(:build_updated) unless transition.loopback?
+      commit_status.pipeline.try do |pipeline|
+        break if transition.loopback?
+
+        if commit_status.complete?
+          ProcessPipelineWorker.perform_async(pipeline.id)
+        end
+
+        UpdatePipelineWorker.perform_async(pipeline.id)
+      end
+
+      true
     end
 
     after_transition [:created, :pending, :running] => :success do |commit_status|
diff --git a/app/workers/process_pipeline_worker.rb b/app/workers/process_pipeline_worker.rb
new file mode 100644
index 00000000000..26ea5f1c24d
--- /dev/null
+++ b/app/workers/process_pipeline_worker.rb
@@ -0,0 +1,10 @@
+class ProcessPipelineWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(pipeline_id)
+    Ci::Pipeline.find_by(id: pipeline_id)
+      .try(:process!)
+  end
+end
diff --git a/app/workers/update_pipeline_worker.rb b/app/workers/update_pipeline_worker.rb
new file mode 100644
index 00000000000..6ef5678073e
--- /dev/null
+++ b/app/workers/update_pipeline_worker.rb
@@ -0,0 +1,10 @@
+class UpdatePipelineWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(pipeline_id)
+    Ci::Pipeline.find_by(id: pipeline_id)
+      .try(:update_status)
+  end
+end
diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb
index 650b410595c..803cbca584d 100644
--- a/db/fixtures/development/14_pipelines.rb
+++ b/db/fixtures/development/14_pipelines.rb
@@ -34,7 +34,7 @@ class Gitlab::Seeder::Pipelines
       rescue ActiveRecord::RecordInvalid
         print 'F'
       ensure
-        pipeline.build_updated
+        pipeline.update_status
       end
     end
   end
diff --git a/spec/features/projects/badges/coverage_spec.rb b/spec/features/projects/badges/coverage_spec.rb
index 5972e7f31c2..01a95bf49ac 100644
--- a/spec/features/projects/badges/coverage_spec.rb
+++ b/spec/features/projects/badges/coverage_spec.rb
@@ -59,7 +59,7 @@ feature 'test coverage badge' do
 
     create(:ci_pipeline, opts).tap do |pipeline|
       yield pipeline
-      pipeline.build_updated
+      pipeline.update_status
     end
   end
 
diff --git a/spec/lib/gitlab/badge/coverage/report_spec.rb b/spec/lib/gitlab/badge/coverage/report_spec.rb
index ab0cce6e091..1547bd3228c 100644
--- a/spec/lib/gitlab/badge/coverage/report_spec.rb
+++ b/spec/lib/gitlab/badge/coverage/report_spec.rb
@@ -100,7 +100,7 @@ describe Gitlab::Badge::Coverage::Report do
 
     create(:ci_pipeline, opts).tap do |pipeline|
       yield pipeline
-      pipeline.build_updated
+      pipeline.update_status
     end
   end
 end
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index cf713684463..26dd95bdfec 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -283,7 +283,7 @@ describe HipchatService, models: true do
     context 'build events' do
       let(:pipeline) { create(:ci_empty_pipeline) }
       let(:build) { create(:ci_build, pipeline: pipeline) }
-      let(:data) { Gitlab::DataBuilder::Build.build(build) }
+      let(:data) { Gitlab::DataBuilder::Build.build(build.reload) }
 
       context 'for failed' do
         before { build.drop }
diff --git a/spec/workers/process_pipeline_worker_spec.rb b/spec/workers/process_pipeline_worker_spec.rb
new file mode 100644
index 00000000000..7b5f98d5763
--- /dev/null
+++ b/spec/workers/process_pipeline_worker_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe ProcessPipelineWorker do
+  describe '#perform' do
+    context 'when pipeline exists' do
+      let(:pipeline) { create(:ci_pipeline) }
+
+      it 'processes pipeline' do
+        expect_any_instance_of(Ci::Pipeline).to receive(:process!)
+
+        described_class.new.perform(pipeline.id)
+      end
+    end
+
+    context 'when pipeline does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end
diff --git a/spec/workers/update_pipeline_worker_spec.rb b/spec/workers/update_pipeline_worker_spec.rb
new file mode 100644
index 00000000000..fadc42b22f0
--- /dev/null
+++ b/spec/workers/update_pipeline_worker_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe UpdatePipelineWorker do
+  describe '#perform' do
+    context 'when pipeline exists' do
+      let(:pipeline) { create(:ci_pipeline) }
+
+      it 'updates pipeline status' do
+        expect_any_instance_of(Ci::Pipeline).to receive(:update_status)
+
+        described_class.new.perform(pipeline.id)
+      end
+    end
+
+    context 'when pipeline does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end