Commit 20fdbbe8 authored by Yorick Peterse's avatar Yorick Peterse Committed by Sean McGivern

Use Goldiloader for handling N+1 queries

Goldiloader (https://github.com/salsify/goldiloader) can eager load
associations automatically. This removes the need for adding "includes"
calls in a variety of different places. This also comes with the added
benefit of not having to eager load data if it's not used.
parent 63ab7c0f
......@@ -441,3 +441,5 @@ gem 'grape_logging', '~> 1.7'
# Asset synchronization
gem 'asset_sync', '~> 2.2.0'
gem 'goldiloader', '~> 2.0'
......@@ -320,6 +320,9 @@ GEM
rubyntlm (~> 0.5)
globalid (0.4.1)
activesupport (>= 4.2.0)
goldiloader (2.0.1)
activerecord (>= 4.2, < 5.2)
activesupport (>= 4.2, < 5.2)
gollum-grit_adapter (1.0.1)
gitlab-grit (~> 2.7, >= 2.7.1)
gollum-lib (4.2.7)
......@@ -1067,6 +1070,7 @@ DEPENDENCIES
gitlab-markup (~> 1.6.2)
gitlab-styles (~> 2.3)
gitlab_omniauth-ldap (~> 2.0.4)
goldiloader (~> 2.0)
gollum-lib (~> 4.2)
gollum-rugged_adapter (~> 0.4.4)
gon (~> 6.1.0)
......
......@@ -15,7 +15,7 @@ module Clusters
belongs_to :user
has_many :cluster_projects, class_name: 'Clusters::Project'
has_many :projects, through: :cluster_projects, class_name: '::Project'
has_many :projects, -> { auto_include(false) }, through: :cluster_projects, class_name: '::Project'
# we force autosave to happen when we save `Cluster` model
has_one :provider_gcp, class_name: 'Clusters::Providers::Gcp', autosave: true
......
......@@ -48,7 +48,7 @@ module Issuable
end
has_many :label_links, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :labels, through: :label_links
has_many :labels, -> { auto_include(false) }, through: :label_links
has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_one :metrics
......
......@@ -102,7 +102,7 @@ module ResolvableDiscussion
yield(notes_relation)
# Set the notes array to the updated notes
@notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
@notes = notes_relation.fresh.auto_include(false).to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
self.class.memoized_values.each do |name|
clear_memoization(name)
......
......@@ -34,7 +34,7 @@ class Issue < ActiveRecord::Base
dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :issue_assignees
has_many :assignees, class_name: "User", through: :issue_assignees
has_many :assignees, -> { auto_include(false) }, class_name: "User", through: :issue_assignees
validates :project, presence: true
......
......@@ -18,8 +18,8 @@ class Label < ActiveRecord::Base
has_many :lists, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :priorities, class_name: 'LabelPriority'
has_many :label_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest'
has_many :issues, -> { auto_include(false) }, through: :label_links, source: :target, source_type: 'Issue'
has_many :merge_requests, -> { auto_include(false) }, through: :label_links, source: :target, source_type: 'MergeRequest'
before_validation :strip_whitespace_from_title_and_color
......
......@@ -179,7 +179,7 @@ class Project < ActiveRecord::Base
has_many :members_and_requesters, as: :source, class_name: 'ProjectMember'
has_many :deploy_keys_projects
has_many :deploy_keys, through: :deploy_keys_projects
has_many :deploy_keys, -> { auto_include(false) }, through: :deploy_keys_projects
has_many :users_star_projects
has_many :starrers, through: :users_star_projects, source: :user
has_many :releases
......@@ -199,7 +199,7 @@ class Project < ActiveRecord::Base
has_one :statistics, class_name: 'ProjectStatistics'
has_one :cluster_project, class_name: 'Clusters::Project'
has_many :clusters, through: :cluster_project, class_name: 'Clusters::Cluster'
has_many :clusters, -> { auto_include(false) }, through: :cluster_project, class_name: 'Clusters::Cluster'
# Container repositories need to remove data from the container registry,
# which is not managed by the DB. Hence we're still using dependent: :destroy
......@@ -225,7 +225,7 @@ class Project < ActiveRecord::Base
has_many :project_deploy_tokens
has_many :deploy_tokens, through: :project_deploy_tokens
has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_many :active_runners, -> { active.auto_include(false) }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_one :auto_devops, class_name: 'ProjectAutoDevops'
has_many :custom_attributes, class_name: 'ProjectCustomAttribute'
......
......@@ -22,7 +22,7 @@ class Todo < ActiveRecord::Base
belongs_to :author, class_name: "User"
belongs_to :note
belongs_to :project
belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
belongs_to :target, -> { auto_include(false) }, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
belongs_to :user
delegate :name, :email, to: :author, prefix: true, allow_nil: true
......
......@@ -96,9 +96,9 @@ class User < ActiveRecord::Base
# Groups
has_many :members
has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember'
has_many :groups, through: :group_members
has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group
has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group
has_many :groups, -> { auto_include(false) }, through: :group_members
has_many :owned_groups, -> { where(members: { access_level: Gitlab::Access::OWNER }).auto_include(false) }, through: :group_members, source: :group
has_many :masters_groups, -> { where(members: { access_level: Gitlab::Access::MASTER }).auto_include(false) }, through: :group_members, source: :group
# Projects
has_many :groups_projects, through: :groups, source: :projects
......
---
title: Add Goldiloader to fix N+1 issues when calculating email recipients
merge_request:
author:
type: performance
......@@ -17,6 +17,17 @@ describe API::PipelineSchedules do
pipeline_schedule.pipelines << build(:ci_pipeline, project: project)
end
def create_pipeline_schedules(count)
create_list(:ci_pipeline_schedule, count, project: project)
.each do |pipeline_schedule|
create(:user).tap do |user|
project.add_developer(user)
pipeline_schedule.update_attributes(owner: user)
end
pipeline_schedule.pipelines << build(:ci_pipeline, project: project)
end
end
it 'returns list of pipeline_schedules' do
get api("/projects/#{project.id}/pipeline_schedules", developer)
......@@ -26,18 +37,14 @@ describe API::PipelineSchedules do
end
it 'avoids N + 1 queries' do
# We need at least two users to trigger a preload for that relation.
create_pipeline_schedules(1)
control_count = ActiveRecord::QueryRecorder.new do
get api("/projects/#{project.id}/pipeline_schedules", developer)
end.count
create_list(:ci_pipeline_schedule, 10, project: project)
.each do |pipeline_schedule|
create(:user).tap do |user|
project.add_developer(user)
pipeline_schedule.update_attributes(owner: user)
end
pipeline_schedule.pipelines << build(:ci_pipeline, project: project)
end
create_pipeline_schedules(10)
expect do
get api("/projects/#{project.id}/pipeline_schedules", developer)
......
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