Commit d17dcf89 authored by Alex Kalderimis's avatar Alex Kalderimis

Use static gap width

This is justified because even at the MAX_ISSUE_COUNT, we would only
ever use 0.1% of the full range width by assuming this value. Extremely
large projects such as gitlab-org/gitlab have on the order of around
100k issues, and would only need 1% of the full range.

We can thus assume such a value as gap_size even allowing for 10x growth
in gitlab's issue numbers, at which point managing locking and
concurrent updates during rebalancing would be a much more challenging
problem.
parent 56b868f6
...@@ -13,7 +13,7 @@ class IssueRebalancingService ...@@ -13,7 +13,7 @@ class IssueRebalancingService
gates = [issue.project, issue.project.group].compact gates = [issue.project, issue.project.group].compact
return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) } return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) }
raise TooManyIssues, "#{issue_count} issues" unless gap_size.present? raise TooManyIssues, "#{issue_count} issues" if issue_count > MAX_ISSUE_COUNT
start = RelativePositioning::START_POSITION - (gaps / 2) * gap_size start = RelativePositioning::START_POSITION - (gaps / 2) * gap_size
...@@ -26,8 +26,6 @@ class IssueRebalancingService ...@@ -26,8 +26,6 @@ class IssueRebalancingService
attr_reader :issue, :base attr_reader :issue, :base
FULL_RANGE = RelativePositioning::MAX_POSITION - RelativePositioning::MIN_POSITION
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def indexed_ids def indexed_ids
base.reorder(:relative_position, :id).pluck(:id).each_with_index base.reorder(:relative_position, :id).pluck(:id).each_with_index
...@@ -62,19 +60,12 @@ class IssueRebalancingService ...@@ -62,19 +60,12 @@ class IssueRebalancingService
end end
def gap_size def gap_size
@gap_size ||= begin # We could try to split the available range over the number of gaps we need,
return if issue_count > MAX_ISSUE_COUNT # but IDEAL_DISTANCE * MAX_ISSUE_COUNT is only 0.1% of the available range,
# so we are guaranteed not to exhaust it by using this static value.
(0.4..0.9).step(0.1) #
.map { |ratio| ratio * FULL_RANGE } # If we raise MAX_ISSUE_COUNT or IDEAL_DISTANCE significantly, this may
.map { |scaled_range| gap_width(scaled_range) } # change!
.select { |gap| gap >= RelativePositioning::MIN_GAP } RelativePositioning::IDEAL_DISTANCE
.max
end
end
# What gap width do we need to use to spread our N gaps out over a given range?
def gap_width(range)
[RelativePositioning::IDEAL_DISTANCE, range / gaps].min.floor
end end
end end
...@@ -98,13 +98,4 @@ RSpec.describe IssueRebalancingService do ...@@ -98,13 +98,4 @@ RSpec.describe IssueRebalancingService do
expect { described_class.new(issue).execute }.to raise_error(described_class::TooManyIssues) expect { described_class.new(issue).execute }.to raise_error(described_class::TooManyIssues)
end end
it 'aborts if we cannot compute a suitable gap' do
issue = project.issues.first
service = described_class.new(issue)
allow(service).to receive(:gap_width).and_return(0)
expect { service.execute }.to raise_error(described_class::TooManyIssues)
end
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