Commit 0bbeff3d authored by Kamil Trzciński's avatar Kamil Trzciński

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
parents f90b5d5d 7f270d04
...@@ -251,9 +251,8 @@ module Ci ...@@ -251,9 +251,8 @@ module Ci
Ci::ProcessPipelineService.new(project, user).execute(self) Ci::ProcessPipelineService.new(project, user).execute(self)
end end
def build_updated def update_status
with_lock do with_lock do
reload
case latest_builds_status case latest_builds_status
when 'pending' then enqueue when 'pending' then enqueue
when 'running' then run when 'running' then run
......
...@@ -84,13 +84,18 @@ class CommitStatus < ActiveRecord::Base ...@@ -84,13 +84,18 @@ class CommitStatus < ActiveRecord::Base
commit_status.update_attributes finished_at: Time.now commit_status.update_attributes finished_at: Time.now
end end
after_transition any => [:success, :failed, :canceled] do |commit_status| after_transition do |commit_status, transition|
commit_status.pipeline.try(:process!) commit_status.pipeline.try do |pipeline|
true break if transition.loopback?
if commit_status.complete?
ProcessPipelineWorker.perform_async(pipeline.id)
end end
after_transition do |commit_status, transition| UpdatePipelineWorker.perform_async(pipeline.id)
commit_status.pipeline.try(:build_updated) unless transition.loopback? end
true
end end
after_transition [:created, :pending, :running] => :success do |commit_status| after_transition [:created, :pending, :running] => :success do |commit_status|
......
class ProcessPipelineWorker
include Sidekiq::Worker
sidekiq_options queue: :default
def perform(pipeline_id)
Ci::Pipeline.find_by(id: pipeline_id)
.try(:process!)
end
end
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
...@@ -34,7 +34,7 @@ class Gitlab::Seeder::Pipelines ...@@ -34,7 +34,7 @@ class Gitlab::Seeder::Pipelines
rescue ActiveRecord::RecordInvalid rescue ActiveRecord::RecordInvalid
print 'F' print 'F'
ensure ensure
pipeline.build_updated pipeline.update_status
end end
end end
end end
......
...@@ -59,7 +59,7 @@ feature 'test coverage badge' do ...@@ -59,7 +59,7 @@ feature 'test coverage badge' do
create(:ci_pipeline, opts).tap do |pipeline| create(:ci_pipeline, opts).tap do |pipeline|
yield pipeline yield pipeline
pipeline.build_updated pipeline.update_status
end end
end end
......
...@@ -100,7 +100,7 @@ describe Gitlab::Badge::Coverage::Report do ...@@ -100,7 +100,7 @@ describe Gitlab::Badge::Coverage::Report do
create(:ci_pipeline, opts).tap do |pipeline| create(:ci_pipeline, opts).tap do |pipeline|
yield pipeline yield pipeline
pipeline.build_updated pipeline.update_status
end end
end end
end end
...@@ -283,7 +283,7 @@ describe HipchatService, models: true do ...@@ -283,7 +283,7 @@ describe HipchatService, models: true do
context 'build events' do context 'build events' do
let(:pipeline) { create(:ci_empty_pipeline) } let(:pipeline) { create(:ci_empty_pipeline) }
let(:build) { create(:ci_build, pipeline: 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 context 'for failed' do
before { build.drop } before { build.drop }
......
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
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment