Commit ee1123de authored by Catalin Irimie's avatar Catalin Irimie

Fix three N+1s in Releases API entity generation

Author and links count queries happened per release, added them
to includes to preload this and remove the N+1.

The links are exposed as links.sorted in the API entity, in order
to preload it, a scope that includes the sort had to be created,
and that included in the preloaded scope to prevent sorting links
for every release as an N+1.

Changelog: performance
parent c765eb69
...@@ -13,7 +13,7 @@ module Types ...@@ -13,7 +13,7 @@ module Types
field :count, GraphQL::INT_TYPE, null: true, method: :assets_count, field :count, GraphQL::INT_TYPE, null: true, method: :assets_count,
description: 'Number of assets of the release.' description: 'Number of assets of the release.'
field :links, Types::ReleaseAssetLinkType.connection_type, null: true, field :links, Types::ReleaseAssetLinkType.connection_type, null: true, method: :sorted_links,
description: 'Asset links of the release.' description: 'Asset links of the release.'
field :sources, Types::ReleaseSourceType.connection_type, null: true, field :sources, Types::ReleaseSourceType.connection_type, null: true,
description: 'Sources of the release.' description: 'Sources of the release.'
......
...@@ -13,6 +13,7 @@ class Release < ApplicationRecord ...@@ -13,6 +13,7 @@ class Release < ApplicationRecord
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
has_many :links, class_name: 'Releases::Link' has_many :links, class_name: 'Releases::Link'
has_many :sorted_links, -> { sorted }, class_name: 'Releases::Link'
has_many :milestone_releases has_many :milestone_releases
has_many :milestones, through: :milestone_releases has_many :milestones, through: :milestone_releases
...@@ -27,7 +28,10 @@ class Release < ApplicationRecord ...@@ -27,7 +28,10 @@ class Release < ApplicationRecord
validates :links, nested_attributes_duplicates: { scope: :release, child_attributes: %i[name url filepath] } validates :links, nested_attributes_duplicates: { scope: :release, child_attributes: %i[name url filepath] }
scope :sorted, -> { order(released_at: :desc) } scope :sorted, -> { order(released_at: :desc) }
scope :preloaded, -> { includes(:evidences, :milestones, project: [:project_feature, :route, { namespace: :route }]) } scope :preloaded, -> {
includes(:author, :evidences, :milestones, :links, :sorted_links,
project: [:project_feature, :route, { namespace: :route }])
}
scope :with_project_and_namespace, -> { includes(project: :namespace) } scope :with_project_and_namespace, -> { includes(project: :namespace) }
scope :recent, -> { sorted.limit(MAX_NUMBER_TO_DISPLAY) } scope :recent, -> { sorted.limit(MAX_NUMBER_TO_DISPLAY) }
scope :without_evidence, -> { left_joins(:evidences).where(::Releases::Evidence.arel_table[:id].eq(nil)) } scope :without_evidence, -> { left_joins(:evidences).where(::Releases::Evidence.arel_table[:id].eq(nil)) }
...@@ -58,8 +62,8 @@ class Release < ApplicationRecord ...@@ -58,8 +62,8 @@ class Release < ApplicationRecord
end end
def assets_count(except: []) def assets_count(except: [])
links_count = links.count links_count = links.size
sources_count = except.include?(:sources) ? 0 : sources.count sources_count = except.include?(:sources) ? 0 : sources.size
links_count + sources_count links_count + sources_count
end end
......
---
title: Fix three N+1s in Releases API entity generation
merge_request: 60189
author:
type: performance
...@@ -28,9 +28,7 @@ module API ...@@ -28,9 +28,7 @@ module API
expose :assets do expose :assets do
expose :assets_count, as: :count expose :assets_count, as: :count
expose :sources, using: Entities::Releases::Source, if: ->(_, _) { can_download_code? } expose :sources, using: Entities::Releases::Source, if: ->(_, _) { can_download_code? }
expose :links, using: Entities::Releases::Link do |release, options| expose :sorted_links, as: :links, using: Entities::Releases::Link
release.links.sorted
end
end end
expose :evidences, using: Entities::Releases::Evidence, expose_nil: false, if: ->(_, _) { can_download_code? } expose :evidences, using: Entities::Releases::Evidence, expose_nil: false, if: ->(_, _) { can_download_code? }
expose :_links do expose :_links do
......
...@@ -113,6 +113,7 @@ releases: ...@@ -113,6 +113,7 @@ releases:
- author - author
- project - project
- links - links
- sorted_links
- milestone_releases - milestone_releases
- milestones - milestones
- evidences - evidences
......
...@@ -54,7 +54,7 @@ RSpec.describe Release do ...@@ -54,7 +54,7 @@ RSpec.describe Release do
end end
describe '#assets_count' do describe '#assets_count' do
subject { release.assets_count } subject { Release.find(release.id).assets_count }
it 'returns the number of sources' do it 'returns the number of sources' do
is_expected.to eq(Gitlab::Workhorse::ARCHIVE_FORMATS.count) is_expected.to eq(Gitlab::Workhorse::ARCHIVE_FORMATS.count)
...@@ -68,7 +68,7 @@ RSpec.describe Release do ...@@ -68,7 +68,7 @@ RSpec.describe Release do
end end
it "excludes sources count when asked" do it "excludes sources count when asked" do
assets_count = release.assets_count(except: [:sources]) assets_count = Release.find(release.id).assets_count(except: [:sources])
expect(assets_count).to eq(1) expect(assets_count).to eq(1)
end end
end end
......
...@@ -136,8 +136,8 @@ RSpec.describe API::Releases do ...@@ -136,8 +136,8 @@ RSpec.describe API::Releases do
get api("/projects/#{project.id}/releases", maintainer) get api("/projects/#{project.id}/releases", maintainer)
end.count end.count
create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer) create_list(:release, 2, :with_evidence, project: project, tag: 'v0.1', author: maintainer)
create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer) create_list(:release, 2, project: project)
expect do expect do
get api("/projects/#{project.id}/releases", maintainer) get api("/projects/#{project.id}/releases", maintainer)
......
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