Commit ecd3104e authored by Shinya Maeda's avatar Shinya Maeda

Optimize Link Merge Request Service

This commit optimizes the Link Merge Request Service
by fetching the list of MRs through notes table without
cross-joining.
parent f474a677
......@@ -288,9 +288,6 @@ class MergeRequest < ApplicationRecord
]
)
end
scope :by_cherry_pick_sha, -> (sha) do
joins(:notes).where(notes: { commit_id: sha })
end
scope :join_project, -> { joins(:target_project) }
scope :join_metrics, -> (target_project_id = nil) do
# Do not join the relation twice
......
......@@ -215,6 +215,10 @@ class Note < ApplicationRecord
def simple_sorts
super.except('name_asc', 'name_desc')
end
def cherry_picked_merge_requests(shas)
where(noteable_type: 'MergeRequest', commit_id: shas).select(:noteable_id)
end
end
# rubocop: disable CodeReuse/ServiceClass
......
......@@ -51,8 +51,15 @@ module Deployments
deployment.link_merge_requests(merge_requests)
picked_merge_requests =
project.merge_requests.by_cherry_pick_sha(slice)
# The cherry picked commits are tracked via `notes.commit_id`
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22209
#
# NOTE: cross-joining `merge_requests` table and `notes` table could
# result in very poor performance because PG planner often uses an
# inappropriate index.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/321032.
mr_ids = project.notes.cherry_picked_merge_requests(slice)
picked_merge_requests = project.merge_requests.id_in(mr_ids)
deployment.link_merge_requests(picked_merge_requests)
end
......
---
title: Optimize searching cherry-picked merge requests for linking deployments
merge_request: 58568
author:
type: performance
......@@ -443,16 +443,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
end
end
describe '.by_cherry_pick_sha' do
it 'returns merge requests that match the given merge commit' do
note = create(:track_mr_picking_note, commit_id: '456abc')
create(:track_mr_picking_note, project: create(:project), commit_id: '456def')
expect(described_class.by_cherry_pick_sha('456abc')).to eq([note.noteable])
end
end
describe '.in_projects' do
it 'returns the merge requests for a set of projects' do
expect(described_class.in_projects(Project.all)).to eq([subject])
......
......@@ -863,6 +863,16 @@ RSpec.describe Note do
end
end
describe '.cherry_picked_merge_requests' do
it 'returns merge requests that match the given merge commit' do
note = create(:track_mr_picking_note, commit_id: '456abc')
create(:track_mr_picking_note, project: create(:project), commit_id: '456def')
expect(MergeRequest.id_in(described_class.cherry_picked_merge_requests('456abc'))).to eq([note.noteable])
end
end
describe '#for_project_snippet?' do
it 'returns true for a project snippet note' do
expect(build(:note_on_project_snippet).for_project_snippet?).to be true
......
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