Commit 47a576d9 authored by Jarka Košanová's avatar Jarka Košanová

List epic related issues by relative_position

- use each_batch insread of find_each
parent 6341e336
...@@ -202,12 +202,15 @@ module EE ...@@ -202,12 +202,15 @@ module EE
::Gitlab::ObjectHierarchy.new(self.where(parent_id: nil)).max_descendants_depth ::Gitlab::ObjectHierarchy.new(self.where(parent_id: nil)).max_descendants_depth
end end
def related_issues(ids:, preload: nil) def related_issues(ids: nil, preload: nil)
::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position, epic_issues.epic_id as epic_id') items = ::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position, epic_issues.epic_id as epic_id')
.joins(:epic_issue) .joins(:epic_issue)
.preload(preload) .preload(preload)
.where("epic_issues.epic_id": ids)
.order('epic_issues.relative_position, epic_issues.id') .order('epic_issues.relative_position, epic_issues.id')
return items unless ids
items.where("epic_issues.epic_id": ids)
end end
end end
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class EpicIssue < ApplicationRecord class EpicIssue < ApplicationRecord
include EpicTreeSorting include EpicTreeSorting
include EachBatch
validates :epic, :issue, presence: true validates :epic, :issue, presence: true
validates :issue, uniqueness: true validates :issue, uniqueness: true
...@@ -13,4 +14,5 @@ class EpicIssue < ApplicationRecord ...@@ -13,4 +14,5 @@ class EpicIssue < ApplicationRecord
alias_attribute :parent, :epic alias_attribute :parent, :epic
scope :in_epic, ->(epic_id) { where(epic_id: epic_id) } scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
scope :related_issues_for_batches, ->(epic_ids) { select(:id, :relative_position).where(epic_id: epic_ids) }
end end
---
title: Order epic related issues by relative_position
merge_request: 23776
author:
type: fixed
...@@ -6,6 +6,7 @@ module Gitlab ...@@ -6,6 +6,7 @@ module Gitlab
class BatchEpicIssuesLoader class BatchEpicIssuesLoader
# this will assure that no more than 100 queries will be done to fetch issues # this will assure that no more than 100 queries will be done to fetch issues
MAX_LOADED_ISSUES = 100_000 MAX_LOADED_ISSUES = 100_000
BATCH_SIZE = 1_000
def initialize(model_id, authorization_filter) def initialize(model_id, authorization_filter)
@model_id = model_id @model_id = model_id
...@@ -14,19 +15,23 @@ module Gitlab ...@@ -14,19 +15,23 @@ module Gitlab
def find def find
BatchLoader::GraphQL.for(@model_id).batch(default_value: []) do |ids, loader| BatchLoader::GraphQL.for(@model_id).batch(default_value: []) do |ids, loader|
issues = ::Epic.related_issues(ids: ids, preload: { project: [:namespace, :project_feature] }) load_issues(loader, ids)
load_issues(loader, issues)
end end
end end
private private
# rubocop: disable CodeReuse/ActiveRecord def load_issues(loader, ids)
def load_issues(loader, issues) issues = ::EpicIssue.related_issues_for_batches(ids)
issues.find_each(batch_size: 1000).with_index do |issue, idx| issues.each_batch(of: BATCH_SIZE, column: 'relative_position') do |batch, idx|
if idx > MAX_LOADED_ISSUES process_batch(loader, batch, idx)
raise Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.' end
end end
def process_batch(loader, batch, idx)
Epic.related_issues(preload: { project: [:namespace, :project_feature] })
.merge(batch.except(:select)).each do |issue|
ensure_limit_not_exceeded!(idx)
loader.call(issue.epic_id) do |memo| loader.call(issue.epic_id) do |memo|
unless memo.is_a?(Gitlab::Graphql::FilterableArray) unless memo.is_a?(Gitlab::Graphql::FilterableArray)
...@@ -38,7 +43,12 @@ module Gitlab ...@@ -38,7 +43,12 @@ module Gitlab
end end
end end
end end
# rubocop: enable CodeReuse/ActiveRecord
def ensure_limit_not_exceeded!(current_index)
if current_index * BATCH_SIZE > MAX_LOADED_ISSUES
raise Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.'
end
end
end end
end end
end end
......
...@@ -26,17 +26,17 @@ describe Resolvers::EpicIssuesResolver do ...@@ -26,17 +26,17 @@ describe Resolvers::EpicIssuesResolver do
describe '#resolve' do describe '#resolve' do
it 'finds all epic issues' do it 'finds all epic issues' do
result = batch_sync(max_queries: 4) { resolve_epic_issues(epic1) } result = batch_sync(max_queries: 6) { resolve_epic_issues(epic1) }
expect(result).to contain_exactly(issue1, issue2) expect(result).to contain_exactly(issue1, issue2)
end end
it 'can batch-resolve epic issues from different epics' do it 'can batch-resolve epic issues from different epics' do
result = batch_sync(max_queries: 4) do result = batch_sync(max_queries: 6) do
[resolve_epic_issues(epic1), resolve_epic_issues(epic2)] [resolve_epic_issues(epic1), resolve_epic_issues(epic2)]
end end
expect(result).to contain_exactly([issue1, issue2], [issue3]) expect(result).to contain_exactly([issue2, issue1], [issue3])
end end
end end
......
...@@ -12,8 +12,10 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do ...@@ -12,8 +12,10 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
let_it_be(:epic2) { create(:epic, group: group) } let_it_be(:epic2) { create(:epic, group: group) }
let_it_be(:issue1) { create(:issue, project: project1) } let_it_be(:issue1) { create(:issue, project: project1) }
let_it_be(:issue2) { create(:issue, project: project2) } let_it_be(:issue2) { create(:issue, project: project2) }
let_it_be(:epic_issue1) { create(:epic_issue, epic: epic1, issue: issue1) } let_it_be(:issue3) { create(:issue, project: project2) }
let_it_be(:epic_issue2) { create(:epic_issue, epic: epic2, issue: issue2) } let_it_be(:epic_issue1) { create(:epic_issue, epic: epic1, issue: issue1, relative_position: 1000) }
let_it_be(:epic_issue2) { create(:epic_issue, epic: epic2, issue: issue2, relative_position: 99999) }
let_it_be(:epic_issue3) { create(:epic_issue, epic: epic2, issue: issue3, relative_position: 1) }
let(:filter) { proc {} } let(:filter) { proc {} }
subject do subject do
...@@ -24,13 +26,13 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do ...@@ -24,13 +26,13 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
end end
it 'only queries once for epic issues' do it 'only queries once for epic issues' do
# 4 queries are done: getting issues and getting projects, # 6 queries are done: 2 queries in EachBatch and then getting issues
# project_features and groups for these issues # and getting projects, project_features and groups for these issues
expect { subject }.not_to exceed_query_limit(4) expect { subject }.not_to exceed_query_limit(6)
end end
it 'returns all epic issues' do it 'returns all epic issues ordered by relative position' do
expect(subject).to eq [[issue1], [issue2]] expect(subject).to eq [[issue1], [issue3, issue2]]
end end
it 'returns an instance of FilterableArray' do it 'returns an instance of FilterableArray' do
...@@ -38,7 +40,7 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do ...@@ -38,7 +40,7 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
end end
it 'raises an error if too many issues are loaded' do it 'raises an error if too many issues are loaded' do
stub_const('Gitlab::Graphql::Loaders::BatchEpicIssuesLoader::MAX_LOADED_ISSUES', 0) stub_const('Gitlab::Graphql::Loaders::BatchEpicIssuesLoader::MAX_LOADED_ISSUES', 2)
expect { subject }.to raise_error Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.' expect { subject }.to raise_error Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.'
end end
......
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