Commit 039a5256 authored by Sean McGivern's avatar Sean McGivern

Move pipeline creation on MR create to Sidekiq

Doing this in the HTTP request means that if pipeline creation is slow,
the whole request can time out - and stop the pipeline being created.

For now, this is behind a feature flag so we can see if there are any
adverse effects.
parent 250c1855
...@@ -6,6 +6,12 @@ module MergeRequests ...@@ -6,6 +6,12 @@ module MergeRequests
event_service.open_mr(merge_request, current_user) event_service.open_mr(merge_request, current_user)
notification_service.new_merge_request(merge_request, current_user) notification_service.new_merge_request(merge_request, current_user)
# https://gitlab.com/gitlab-org/gitlab/issues/208813
if ::Feature.enabled?(:create_merge_request_pipelines_in_sidekiq, project)
create_pipeline_for(merge_request, current_user)
merge_request.update_head_pipeline
end
merge_request.diffs(include_stats: false).write_cache merge_request.diffs(include_stats: false).write_cache
merge_request.create_cross_references!(current_user) merge_request.create_cross_references!(current_user)
end end
......
...@@ -20,8 +20,13 @@ module MergeRequests ...@@ -20,8 +20,13 @@ module MergeRequests
todo_service.new_merge_request(issuable, current_user) todo_service.new_merge_request(issuable, current_user)
issuable.cache_merge_request_closes_issues!(current_user) issuable.cache_merge_request_closes_issues!(current_user)
create_pipeline_for(issuable, current_user)
issuable.update_head_pipeline # https://gitlab.com/gitlab-org/gitlab/issues/208813
unless ::Feature.enabled?(:create_merge_request_pipelines_in_sidekiq, project)
create_pipeline_for(issuable, current_user)
issuable.update_head_pipeline
end
Gitlab::UsageDataCounters::MergeRequestCounter.count(:create) Gitlab::UsageDataCounters::MergeRequestCounter.count(:create)
link_lfs_objects(issuable) link_lfs_objects(issuable)
......
...@@ -19,27 +19,53 @@ describe MergeRequests::AfterCreateService do ...@@ -19,27 +19,53 @@ describe MergeRequests::AfterCreateService do
end end
it 'creates a merge request open event' do it 'creates a merge request open event' do
expect(event_service).to receive(:open_mr).with(merge_request, merge_request.author) expect(event_service)
.to receive(:open_mr).with(merge_request, merge_request.author)
after_create_service.execute(merge_request) after_create_service.execute(merge_request)
end end
it 'creates a new merge request notification' do it 'creates a new merge request notification' do
expect(notification_service).to receive(:new_merge_request).with(merge_request, merge_request.author) expect(notification_service)
.to receive(:new_merge_request).with(merge_request, merge_request.author)
after_create_service.execute(merge_request) after_create_service.execute(merge_request)
end end
it 'writes diffs to the cache' do it 'writes diffs to the cache' do
expect(merge_request).to receive_message_chain(:diffs, :write_cache) expect(merge_request)
.to receive_message_chain(:diffs, :write_cache)
after_create_service.execute(merge_request) after_create_service.execute(merge_request)
end end
it 'creates cross references' do it 'creates cross references' do
expect(merge_request).to receive(:create_cross_references!).with(merge_request.author) expect(merge_request)
.to receive(:create_cross_references!).with(merge_request.author)
after_create_service.execute(merge_request) after_create_service.execute(merge_request)
end end
it 'creates a pipeline and updates the HEAD pipeline' do
expect(after_create_service)
.to receive(:create_pipeline_for).with(merge_request, merge_request.author)
expect(merge_request).to receive(:update_head_pipeline)
after_create_service.execute(merge_request)
end
# https://gitlab.com/gitlab-org/gitlab/issues/208813
context 'when the create_merge_request_pipelines_in_sidekiq flag is disabled' do
before do
stub_feature_flags(create_merge_request_pipelines_in_sidekiq: false)
end
it 'does not create a pipeline or update the HEAD pipeline' do
expect(after_create_service).not_to receive(:create_pipeline_for)
expect(merge_request).not_to receive(:update_head_pipeline)
after_create_service.execute(merge_request)
end
end
end end
end end
...@@ -129,7 +129,23 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -129,7 +129,23 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end end
end end
context 'when head pipelines already exist for merge request source branch' do # https://gitlab.com/gitlab-org/gitlab/issues/208813
context 'when the create_merge_request_pipelines_in_sidekiq flag is disabled' do
before do
stub_feature_flags(create_merge_request_pipelines_in_sidekiq: false)
end
it 'creates a pipeline and updates the HEAD pipeline' do
expect(service).to receive(:create_pipeline_for)
expect_next_instance_of(MergeRequest) do |merge_request|
expect(merge_request).to receive(:update_head_pipeline)
end
service.execute
end
end
context 'when head pipelines already exist for merge request source branch', :sidekiq_inline do
let(:shas) { project.repository.commits(opts[:source_branch], limit: 2).map(&:id) } let(:shas) { project.repository.commits(opts[:source_branch], limit: 2).map(&:id) }
let!(:pipeline_1) { create(:ci_pipeline, project: project, ref: opts[:source_branch], project_id: project.id, sha: shas[1]) } let!(:pipeline_1) { create(:ci_pipeline, project: project, ref: opts[:source_branch], project_id: project.id, sha: shas[1]) }
let!(:pipeline_2) { create(:ci_pipeline, project: project, ref: opts[:source_branch], project_id: project.id, sha: shas[0]) } let!(:pipeline_2) { create(:ci_pipeline, project: project, ref: opts[:source_branch], project_id: project.id, sha: shas[0]) }
...@@ -175,7 +191,7 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -175,7 +191,7 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end end
end end
describe 'Pipelines for merge requests' do describe 'Pipelines for merge requests', :sidekiq_inline do
before do before do
stub_ci_pipeline_yaml_file(YAML.dump(config)) stub_ci_pipeline_yaml_file(YAML.dump(config))
end end
...@@ -216,7 +232,9 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -216,7 +232,9 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
target_project.add_maintainer(user) target_project.add_maintainer(user)
end end
it 'create legacy detached merge request pipeline for fork merge request', :sidekiq_might_not_need_inline do it 'create legacy detached merge request pipeline for fork merge request' do
merge_request.reload
expect(merge_request.actual_head_pipeline) expect(merge_request.actual_head_pipeline)
.to be_legacy_detached_merge_request_pipeline .to be_legacy_detached_merge_request_pipeline
end end
...@@ -228,6 +246,8 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -228,6 +246,8 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end end
it 'create legacy detached merge request pipeline for non-fork merge request' do it 'create legacy detached merge request pipeline for non-fork merge request' do
merge_request.reload
expect(merge_request.actual_head_pipeline) expect(merge_request.actual_head_pipeline)
.to be_legacy_detached_merge_request_pipeline .to be_legacy_detached_merge_request_pipeline
end end
...@@ -262,6 +282,8 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -262,6 +282,8 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end end
it 'sets the latest detached merge request pipeline as the head pipeline' do it 'sets the latest detached merge request pipeline as the head pipeline' do
merge_request.reload
expect(merge_request.actual_head_pipeline).to be_merge_request_event expect(merge_request.actual_head_pipeline).to be_merge_request_event
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