Commit 7707fe87 authored by Igor Drozdov's avatar Igor Drozdov

Refresh only existing MRs on push

When N refs are pushed in a single batch N refresh jobs are scheduled
This commit checks whether a merge request for a branch exists
before scheduling a job
parent 2babea36
......@@ -36,6 +36,8 @@ module Git
# Update merge requests that may be affected by this push. A new branch
# could cause the last commit of a merge request to change.
def enqueue_update_mrs
return if params[:merge_request_branches]&.exclude?(branch_name)
UpdateMergeRequestsWorker.perform_async(
project.id,
current_user.id,
......
......@@ -42,6 +42,7 @@ module Git
push_service_class = push_service_class_for(ref_type)
create_bulk_push_event = changes.size > Gitlab::CurrentSettings.push_event_activities_limit
merge_request_branches = merge_request_branches_for(changes)
changes.each do |change|
push_service_class.new(
......@@ -49,6 +50,7 @@ module Git
current_user,
change: change,
push_options: params[:push_options],
merge_request_branches: merge_request_branches,
create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project),
execute_project_hooks: execute_project_hooks,
create_push_event: !create_bulk_push_event
......@@ -71,5 +73,11 @@ module Git
Git::BranchPushService
end
def merge_request_branches_for(changes)
return if Feature.disabled?(:refresh_only_existing_merge_requests_on_push, default_enabled: true)
@merge_requests_branches ||= MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute
end
end
end
# frozen_string_literal: true
module MergeRequests
class PushedBranchesService < MergeRequests::BaseService
include ::Gitlab::Utils::StrongMemoize
# Skip moving this logic into models since it's too specific
# rubocop: disable CodeReuse/ActiveRecord
def execute
return [] if branch_names.blank?
source_branches = project.source_of_merge_requests.opened
.from_source_branches(branch_names).pluck(:source_branch)
target_branches = project.merge_requests.opened
.by_target_branch(branch_names).distinct.pluck(:target_branch)
source_branches.concat(target_branches).to_set
end
# rubocop: enable CodeReuse/ActiveRecord
private
def branch_names
strong_memoize(:branch_names) do
params[:changes].map do |change|
Gitlab::Git.branch_name(change[:ref])
end.compact
end
end
end
end
---
title: Refresh only existing MRs on push
merge_request: 29420
author:
type: performance
......@@ -160,6 +160,49 @@ describe Git::ProcessRefChangesService do
let(:ref_prefix) { 'refs/heads' }
it_behaves_like 'service for processing ref changes', Git::BranchPushService
context 'when there are merge requests associated with branches' do
let(:tag_changes) do
[
{ index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "refs/tags/v10.0.0" }
]
end
let(:branch_changes) do
[
{ index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "#{ref_prefix}/create1" },
{ index: 1, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789013', ref: "#{ref_prefix}/create2" },
{ index: 2, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789014', ref: "#{ref_prefix}/create3" }
]
end
let(:git_changes) { double(branch_changes: branch_changes, tag_changes: tag_changes) }
it 'schedules job for existing merge requests' do
expect_next_instance_of(MergeRequests::PushedBranchesService) do |service|
expect(service).to receive(:execute).and_return(%w(create1 create2))
end
expect(UpdateMergeRequestsWorker).to receive(:perform_async)
.with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789012', "#{ref_prefix}/create1").ordered
expect(UpdateMergeRequestsWorker).to receive(:perform_async)
.with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789013', "#{ref_prefix}/create2").ordered
expect(UpdateMergeRequestsWorker).not_to receive(:perform_async)
.with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789014', "#{ref_prefix}/create3").ordered
subject.execute
end
context 'refresh_only_existing_merge_requests_on_push disabled' do
before do
stub_feature_flags(refresh_only_existing_merge_requests_on_push: false)
end
it 'refreshes all merge requests' do
expect(UpdateMergeRequestsWorker).to receive(:perform_async).exactly(3).times
subject.execute
end
end
end
end
context 'tag changes' do
......
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::PushedBranchesService do
let(:project) { create(:project) }
let!(:service) { described_class.new(project, nil, changes: pushed_branches) }
context 'when branches pushed' do
let(:pushed_branches) do
%w(branch1 branch2 extra1 extra2 extra3).map do |branch|
{ ref: "refs/heads/#{branch}" }
end
end
it 'returns only branches which have a merge request' do
create(:merge_request, source_branch: 'branch1', source_project: project)
create(:merge_request, source_branch: 'branch2', source_project: project)
create(:merge_request, target_branch: 'branch2', source_project: project)
create(:merge_request, :closed, target_branch: 'extra1', source_project: project)
create(:merge_request, source_branch: 'extra2')
expect(service.execute).to contain_exactly('branch1', 'branch2')
end
end
context 'when tags pushed' do
let(:pushed_branches) do
%w(v10.0.0 v11.0.2 v12.1.0).map do |branch|
{ ref: "refs/tags/#{branch}" }
end
end
it 'returns empty result without any SQL query performed' do
control_count = ActiveRecord::QueryRecorder.new do
expect(service.execute).to be_empty
end.count
expect(control_count).to be_zero
end
end
end
......@@ -352,6 +352,9 @@ describe PostReceive do
it "enqueues a UpdateMergeRequestsWorker job" do
allow(Project).to receive(:find_by).and_return(project)
expect_next_instance_of(MergeRequests::PushedBranchesService) do |service|
expect(service).to receive(:execute).and_return(%w(tést))
end
expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args)
......
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