Commit 299e26be authored by Alex Kalderimis's avatar Alex Kalderimis

Add issue rebalancing if needed to issue_placement_worker

This uses the logic for issue rebalancing in the issue base services.
parent ac4357cd
...@@ -17,8 +17,6 @@ module Issues ...@@ -17,8 +17,6 @@ module Issues
Issues::CloseService Issues::CloseService
end end
private
NO_REBALANCING_NEEDED = ((RelativePositioning::MIN_POSITION * 0.9999)..(RelativePositioning::MAX_POSITION * 0.9999)).freeze NO_REBALANCING_NEEDED = ((RelativePositioning::MIN_POSITION * 0.9999)..(RelativePositioning::MAX_POSITION * 0.9999)).freeze
def rebalance_if_needed(issue) def rebalance_if_needed(issue)
...@@ -32,6 +30,8 @@ module Issues ...@@ -32,6 +30,8 @@ module Issues
IssueRebalancingWorker.perform_async(nil, issue.project_id) IssueRebalancingWorker.perform_async(nil, issue.project_id)
end end
private
def create_assignee_note(issue, old_assignees) def create_assignee_note(issue, old_assignees)
SystemNoteService.change_issuable_assignees( SystemNoteService.change_issuable_assignees(
issue, issue.project, current_user, old_assignees) issue, issue.project, current_user, old_assignees)
......
...@@ -27,6 +27,7 @@ class IssuePlacementWorker ...@@ -27,6 +27,7 @@ class IssuePlacementWorker
.limit(QUERY_LIMIT) .limit(QUERY_LIMIT)
Issue.move_nulls_to_end(to_place.to_a.reverse) Issue.move_nulls_to_end(to_place.to_a.reverse)
Issues::BaseService.new(nil).rebalance_if_needed(to_place.max_by(&:relative_position))
rescue RelativePositioning::NoSpaceLeft => e rescue RelativePositioning::NoSpaceLeft => e
Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id) Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id)
IssueRebalancingWorker.perform_async(nil, issue.project_id) IssueRebalancingWorker.perform_async(nil, issue.project_id)
......
...@@ -5,16 +5,19 @@ require 'spec_helper' ...@@ -5,16 +5,19 @@ require 'spec_helper'
RSpec.describe IssuePlacementWorker do RSpec.describe IssuePlacementWorker do
describe '#perform' do describe '#perform' do
let_it_be(:time) { Time.now.utc } let_it_be(:time) { Time.now.utc }
let_it_be(:project) { create_default(:project) } let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, relative_position: nil, created_at: time) } let_it_be(:author) { create(:user) }
let_it_be(:issue_a) { create(:issue, relative_position: nil, created_at: time - 1.minute) } let_it_be(:common_attrs) { { author: author, project: project } }
let_it_be(:issue_b) { create(:issue, relative_position: nil, created_at: time - 6.minutes) } let_it_be(:unplaced) { common_attrs.merge(relative_position: nil) }
let_it_be(:issue_c) { create(:issue, relative_position: nil, created_at: time + 1.minute) } let_it_be(:issue) { create(:issue, **unplaced, created_at: time) }
let_it_be(:issue_d) { create(:issue, relative_position: nil, created_at: time + 6.minutes) } let_it_be(:issue_a) { create(:issue, **unplaced, created_at: time - 1.minute) }
let_it_be(:issue_e) { create(:issue, relative_position: 10, created_at: time + 3.minutes) } let_it_be(:issue_b) { create(:issue, **unplaced, created_at: time - 2.minutes) }
let_it_be(:issue_f) { create(:issue, relative_position: nil, created_at: time + 1.minute) } let_it_be(:issue_c) { create(:issue, **unplaced, created_at: time + 1.minute) }
let_it_be(:issue_d) { create(:issue, **unplaced, created_at: time + 2.minutes) }
let_it_be(:issue_e) { create(:issue, **common_attrs, relative_position: 10, created_at: time + 1.minute) }
let_it_be(:issue_f) { create(:issue, **unplaced, created_at: time + 1.minute) }
let_it_be(:irrelevant) { create(:issue, project: create(:project), relative_position: nil, created_at: time) } let_it_be(:irrelevant) { create(:issue, relative_position: nil, created_at: time) }
it 'places all issues created at most 5 minutes before this one at the end, most recent last' do it 'places all issues created at most 5 minutes before this one at the end, most recent last' do
expect do expect do
...@@ -26,10 +29,18 @@ RSpec.describe IssuePlacementWorker do ...@@ -26,10 +29,18 @@ RSpec.describe IssuePlacementWorker do
expect(project.issues.where(relative_position: nil)).not_to exist expect(project.issues.where(relative_position: nil)).not_to exist
end end
it 'schedules rebalancing if needed' do
issue_a.update!(relative_position: RelativePositioning::MAX_POSITION)
expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
described_class.new.perform(issue.id)
end
it 'limits the sweep to QUERY_LIMIT records' do it 'limits the sweep to QUERY_LIMIT records' do
issues = create_list(:issue, described_class::QUERY_LIMIT + 1, relative_position: nil) create_list(:issue, described_class::QUERY_LIMIT - 5, **unplaced)
described_class.new.perform(issues.first.id) described_class.new.perform(issue.id)
expect(project.issues.where(relative_position: nil)).to exist expect(project.issues.where(relative_position: nil)).to exist
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