Commit cbc50b76 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Optimize query for issue neighbors

Fixes query timeouts when ordering issues in issue boards

Changelog: fixed
parent 43a6b985
...@@ -168,6 +168,24 @@ module RelativePositioning ...@@ -168,6 +168,24 @@ module RelativePositioning
self.relative_position = MIN_POSITION self.relative_position = MIN_POSITION
end end
def next_object_by_relative_position(ignoring: nil, order: :asc)
relation = relative_positioning_scoped_items(ignoring: ignoring).reorder(relative_position: order)
relation = if order == :asc
relation.where(self.class.arel_table[:relative_position].gt(relative_position))
else
relation.where(self.class.arel_table[:relative_position].lt(relative_position))
end
relation.first
end
def relative_positioning_scoped_items(ignoring: nil)
relation = self.class.relative_positioning_query_base(self)
relation = exclude_self(relation, excluded: ignoring) if ignoring.present?
relation
end
# This method is used during rebalancing - override it to customise the update # This method is used during rebalancing - override it to customise the update
# logic: # logic:
def update_relative_siblings(relation, range, delta) def update_relative_siblings(relation, range, delta)
......
...@@ -229,9 +229,37 @@ class Issue < ApplicationRecord ...@@ -229,9 +229,37 @@ class Issue < ApplicationRecord
end end
end end
def next_object_by_relative_position(ignoring: nil, order: :asc)
return super unless Feature.enabled?(:optimized_issue_neighbor_queries, project, default_enabled: :yaml)
array_mapping_scope = -> (id_expression) do
relation = Issue.where(Issue.arel_table[:project_id].eq(id_expression))
if order == :asc
relation.where(Issue.arel_table[:relative_position].gt(relative_position))
else
relation.where(Issue.arel_table[:relative_position].lt(relative_position))
end
end
relation = Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Issue.order(relative_position: order, id: order),
array_scope: relative_positioning_parent_projects,
array_mapping_scope: array_mapping_scope,
finder_query: -> (_, id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) }
).execute
relation = exclude_self(relation, excluded: ignoring) if ignoring.present?
relation.take
end
def relative_positioning_parent_projects
project.group&.root_ancestor&.all_projects&.select(:id) || Project.id_in(project).select(:id)
end
def self.relative_positioning_query_base(issue) def self.relative_positioning_query_base(issue)
projects = issue.project.group&.root_ancestor&.all_projects || issue.project in_projects(issue.relative_positioning_parent_projects)
in_projects(projects)
end end
def self.relative_positioning_parent_column def self.relative_positioning_parent_column
......
---
name: optimized_issue_neighbor_queries
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76073
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345921
milestone: '14.6'
type: development
group: group::project management
default_enabled: false
...@@ -66,19 +66,11 @@ module Gitlab ...@@ -66,19 +66,11 @@ module Gitlab
end end
def lhs_neighbour def lhs_neighbour
scoped_items neighbour(object.next_object_by_relative_position(ignoring: ignoring, order: :desc))
.where('relative_position < ?', relative_position)
.reorder(relative_position: :desc)
.first
.then { |x| neighbour(x) }
end end
def rhs_neighbour def rhs_neighbour
scoped_items neighbour(object.next_object_by_relative_position(ignoring: ignoring, order: :asc))
.where('relative_position > ?', relative_position)
.reorder(relative_position: :asc)
.first
.then { |x| neighbour(x) }
end end
def neighbour(item) def neighbour(item)
...@@ -87,12 +79,6 @@ module Gitlab ...@@ -87,12 +79,6 @@ module Gitlab
self.class.new(item, range, ignoring: ignoring) self.class.new(item, range, ignoring: ignoring)
end end
def scoped_items
r = model_class.relative_positioning_query_base(object)
r = object.exclude_self(r, excluded: ignoring) if ignoring.present?
r
end
def calculate_relative_position(calculation) def calculate_relative_position(calculation)
# When calculating across projects, this is much more efficient than # When calculating across projects, this is much more efficient than
# MAX(relative_position) without the GROUP BY, due to index usage: # MAX(relative_position) without the GROUP BY, due to index usage:
...@@ -186,6 +172,10 @@ module Gitlab ...@@ -186,6 +172,10 @@ module Gitlab
Gap.new(gap.first, gap.second || default_end) Gap.new(gap.first, gap.second || default_end)
end end
def scoped_items
object.relative_positioning_scoped_items(ignoring: ignoring)
end
def relative_position def relative_position
object.relative_position object.relative_position
end end
......
...@@ -1317,11 +1317,29 @@ RSpec.describe Issue do ...@@ -1317,11 +1317,29 @@ RSpec.describe Issue do
let_it_be(:issue1) { create(:issue, project: project, relative_position: nil) } let_it_be(:issue1) { create(:issue, project: project, relative_position: nil) }
let_it_be(:issue2) { create(:issue, project: project, relative_position: nil) } let_it_be(:issue2) { create(:issue, project: project, relative_position: nil) }
context 'when optimized_issue_neighbor_queries is enabled' do
before do
stub_feature_flags(optimized_issue_neighbor_queries: true)
end
it_behaves_like "a class that supports relative positioning" do
let_it_be(:project) { reusable_project }
let(:factory) { :issue }
let(:default_params) { { project: project } }
end
end
context 'when optimized_issue_neighbor_queries is disabled' do
before do
stub_feature_flags(optimized_issue_neighbor_queries: false)
end
it_behaves_like "a class that supports relative positioning" do it_behaves_like "a class that supports relative positioning" do
let_it_be(:project) { reusable_project } let_it_be(:project) { reusable_project }
let(:factory) { :issue } let(:factory) { :issue }
let(:default_params) { { project: project } } let(:default_params) { { project: project } }
end end
end
it 'is not blocked for repositioning by default' do it 'is not blocked for repositioning by default' do
expect(issue1.blocked_for_repositioning?).to eq(false) expect(issue1.blocked_for_repositioning?).to eq(false)
......
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