Reduce SQL queries needed to load open merge requests

The SQL queries and memory allocation in MergeRequests::RefreshService
is dominated by queries for Project and Route loads. On staging, the
absence of an inverse relationship caused Rails to make over 1100
extraneous SQL queries for the www-gitlab-com repository.

Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/49703
This commit is contained in:
Stan Hu 2018-10-30 16:41:49 -07:00
parent 571e651b21
commit d4ef4ad752
5 changed files with 25 additions and 2 deletions

View file

@ -181,7 +181,7 @@ class Project < ActiveRecord::Base
has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
# Merge Requests for target project should be removed with it # Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id' has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project
has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest' has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
has_many :issues has_many :issues
has_many :labels, class_name: 'ProjectLabel' has_many :labels, class_name: 'ProjectLabel'

View file

@ -0,0 +1,5 @@
---
title: Reduce SQL queries needed to load open merge requests
merge_request: 22709
author:
type: performance

View file

@ -22,7 +22,7 @@ module Gitlab
# additional work that is strictly necessary. # additional work that is strictly necessary.
merge_request_id = insert_and_return_id(attributes, project.merge_requests) merge_request_id = insert_and_return_id(attributes, project.merge_requests)
merge_request = project.merge_requests.find(merge_request_id) merge_request = project.merge_requests.reload.find(merge_request_id)
# We use .insert_and_return_id which effectively disables all callbacks. # We use .insert_and_return_id which effectively disables all callbacks.
# Trigger iid logic here to make sure we track internal id values consistently. # Trigger iid logic here to make sure we track internal id values consistently.

View file

@ -13,6 +13,20 @@ describe MergeRequest do
it { is_expected.to belong_to(:merge_user).class_name("User") } it { is_expected.to belong_to(:merge_user).class_name("User") }
it { is_expected.to belong_to(:assignee) } it { is_expected.to belong_to(:assignee) }
it { is_expected.to have_many(:merge_request_diffs) } it { is_expected.to have_many(:merge_request_diffs) }
context 'for forks' do
let!(:project) { create(:project) }
let!(:fork) { fork_project(project) }
let!(:merge_request) { create(:merge_request, target_project: project, source_project: fork) }
it 'does not load another project due to inverse relationship' do
expect(project.merge_requests.first.target_project.object_id).to eq(project.object_id)
end
it 'finds the associated merge request' do
expect(project.merge_requests.find(merge_request.id)).to eq(merge_request)
end
end
end end
describe '#squash_in_progress?' do describe '#squash_in_progress?' do

View file

@ -88,6 +88,10 @@ describe Project do
it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:project_deploy_tokens) }
it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) }
it 'has an inverse relationship with merge requests' do
expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project)
end
context 'after initialized' do context 'after initialized' do
it "has a project_feature" do it "has a project_feature" do
expect(described_class.new.project_feature).to be_present expect(described_class.new.project_feature).to be_present