Commit eb15e4df authored by Sean McGivern's avatar Sean McGivern

Speed up setting of relative position

1. When every issue has a relative position set, we don't need to
   perform any updates, or calculate the maximum position in the parent.
2. If we do need to calculate the maximum position in the parent, many
   parents (specifically, groups with lots of projects) leads to a slow
   query where only the index on issues.relative_position is used, not
   the index on issues.project_id. Adding the GROUP BY forces Postgres
   to use both indices.
parent 3b601b82
...@@ -14,10 +14,12 @@ module RelativePositioning ...@@ -14,10 +14,12 @@ module RelativePositioning
class_methods do class_methods do
def move_to_end(objects) def move_to_end(objects)
parent_ids = objects.map(&:parent_ids).flatten.uniq
max_relative_position = in_parents(parent_ids).maximum(:relative_position) || START_POSITION
objects = objects.reject(&:relative_position) objects = objects.reject(&:relative_position)
return if objects.empty?
max_relative_position = objects.first.max_relative_position
self.transaction do self.transaction do
objects.each do |object| objects.each do |object|
relative_position = position_between(max_relative_position, MAX_POSITION) relative_position = position_between(max_relative_position, MAX_POSITION)
...@@ -55,22 +57,21 @@ module RelativePositioning ...@@ -55,22 +57,21 @@ module RelativePositioning
end end
end end
def min_relative_position def min_relative_position(&block)
self.class.in_parents(parent_ids).minimum(:relative_position) calculate_relative_position('MIN', &block)
end end
def max_relative_position def max_relative_position(&block)
self.class.in_parents(parent_ids).maximum(:relative_position) calculate_relative_position('MAX', &block)
end end
def prev_relative_position def prev_relative_position
prev_pos = nil prev_pos = nil
if self.relative_position if self.relative_position
prev_pos = self.class prev_pos = max_relative_position do |relation|
.in_parents(parent_ids) relation.where('relative_position < ?', self.relative_position)
.where('relative_position < ?', self.relative_position) end
.maximum(:relative_position)
end end
prev_pos prev_pos
...@@ -80,10 +81,9 @@ module RelativePositioning ...@@ -80,10 +81,9 @@ module RelativePositioning
next_pos = nil next_pos = nil
if self.relative_position if self.relative_position
next_pos = self.class next_pos = min_relative_position do |relation|
.in_parents(parent_ids) relation.where('relative_position > ?', self.relative_position)
.where('relative_position > ?', self.relative_position) end
.minimum(:relative_position)
end end
next_pos next_pos
...@@ -165,4 +165,22 @@ module RelativePositioning ...@@ -165,4 +165,22 @@ module RelativePositioning
status status
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables # rubocop:enable Gitlab/ModuleWithInstanceVariables
def calculate_relative_position(calculation)
# When calculating across projects, this is much more efficient than
# MAX(relative_position) without the GROUP BY, due to index usage:
# https://gitlab.com/gitlab-org/gitlab-ce/issues/54276#note_119340977
relation = self.class
.in_parents(parent_ids)
.order(Gitlab::Database.nulls_last_order('position', 'DESC'))
.limit(1)
.group(self.class.parent_column)
relation = yield relation if block_given?
relation
.pluck(self.class.parent_column, "#{calculation}(relative_position) AS position")
.first&.
last
end
end end
...@@ -99,6 +99,10 @@ class Issue < ActiveRecord::Base ...@@ -99,6 +99,10 @@ class Issue < ActiveRecord::Base
alias_method :in_parents, :in_projects alias_method :in_parents, :in_projects
end end
def self.parent_column
:project_id
end
def self.reference_prefix def self.reference_prefix
'#' '#'
end end
......
---
title: Speed up issue board lists in groups with many projects
merge_request:
author:
type: performance
...@@ -14,6 +14,14 @@ describe RelativePositioning do ...@@ -14,6 +14,14 @@ describe RelativePositioning do
expect(issue.prev_relative_position).to eq nil expect(issue.prev_relative_position).to eq nil
expect(issue1.next_relative_position).to eq nil expect(issue1.next_relative_position).to eq nil
end end
it 'does not perform any moves if all issues have their relative_position set' do
issue.update!(relative_position: 1)
expect(issue).not_to receive(:save)
Issue.move_to_end([issue])
end
end end
describe '#max_relative_position' do describe '#max_relative_position' do
......
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