Have the rebalancing worker take project ID

This has the following benefit: automatic deduplication of requests.

This will not serve all deduplication purposes, but it will catch most
of them (imports into projects, large busy projects).

Since this changes the meaning of the arguments, we have added a new
positional argument.

app/models/issue.rb

@@ -445,7 +445,7 @@ class Issue < ApplicationRecord
     super
   rescue ActiveRecord::QueryCanceled => e
     # Symptom of running out of space - schedule rebalancing
-    IssueRebalancingWorker.perform_async(id)
+    IssueRebalancingWorker.perform_async(nil, project_id)
     raise e
   end
 
@@ -453,7 +453,7 @@ class Issue < ApplicationRecord
     super
   rescue ActiveRecord::QueryCanceled => e
     # Symptom of running out of space - schedule rebalancing
-    IssueRebalancingWorker.perform_async(id)
+    IssueRebalancingWorker.perform_async(nil, project_id)
     raise e
   end
 end

app/services/issues/base_service.rb

@@ -29,7 +29,7 @@ module Issues
       gates = [issue.project, issue.project.group].compact
       return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) }
 
-      IssueRebalancingWorker.perform_async(issue.id)
+      IssueRebalancingWorker.perform_async(nil, issue.project_id)
     end
 
     def create_assignee_note(issue, old_assignees)

app/workers/issue_rebalancing_worker.rb

@@ -7,11 +7,14 @@ class IssueRebalancingWorker
   urgency :low
   feature_category :issue_tracking
 
-  def perform(issue_id)
-    issue = Issue.find(issue_id)
+  def perform(ignore = nil, project_id = nil)
+    return if project_id.nil?
+
+    project = Project.find(project_id)
+    issue = project.issues.first # All issues are equivalent as far as we are concerned
 
     IssueRebalancingService.new(issue).execute
   rescue ActiveRecord::RecordNotFound, IssueRebalancingService::TooManyIssues => e
-    Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id)
+    Gitlab::ErrorTracking.log_exception(e, project_id: project_id)
   end
 end

spec/models/issue_spec.rb

@@ -1196,7 +1196,7 @@ RSpec.describe Issue do
       it 'schedules rebalancing if we time-out when finding a gap' do
         lhs = build_stubbed(:issue, relative_position: 99, project: project)
         to_move = build(:issue, project: project)
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         expect { to_move.move_between(lhs, issue) }.to raise_error(ActiveRecord::QueryCanceled)
       end
@@ -1205,7 +1205,7 @@ RSpec.describe Issue do
     describe '#find_next_gap_after' do
       it 'schedules rebalancing if we time-out when finding a gap' do
         allow(issue).to receive(:find_next_gap) { raise ActiveRecord::QueryCanceled }
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         expect { issue.move_sequence_after }.to raise_error(ActiveRecord::QueryCanceled)
       end

spec/services/issues/create_service_spec.rb

@@ -77,7 +77,7 @@ RSpec.describe Issues::CreateService do
 
       it 'rebalances if needed' do
         create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(Integer)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
       end
@@ -86,7 +86,7 @@ RSpec.describe Issues::CreateService do
         stub_feature_flags(rebalance_issues: false)
 
         create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
-        expect(IssueRebalancingWorker).not_to receive(:perform_async).with(Integer)
+        expect(IssueRebalancingWorker).not_to receive(:perform_async)
 
         expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
       end
@@ -95,7 +95,7 @@ RSpec.describe Issues::CreateService do
         stub_feature_flags(rebalance_issues: project)
 
         create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(Integer)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
       end

spec/services/issues/update_service_spec.rb

@@ -126,7 +126,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).not_to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).not_to receive(:perform_async)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -142,7 +142,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -156,7 +156,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -170,7 +170,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)

spec/workers/issue_rebalancing_worker_spec.rb

@@ -10,23 +10,30 @@ RSpec.describe IssueRebalancingWorker do
       service = double(execute: nil)
       expect(IssueRebalancingService).to receive(:new).with(issue).and_return(service)
 
-      described_class.new.perform(issue.id)
+      described_class.new.perform(nil, issue.project_id)
     end
 
     it 'anticipates the inability to find the issue' do
-      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(ActiveRecord::RecordNotFound, include(issue_id: -1))
+      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(ActiveRecord::RecordNotFound, include(project_id: -1))
       expect(IssueRebalancingService).not_to receive(:new)
 
-      described_class.new.perform(-1)
+      described_class.new.perform(nil, -1)
     end
 
     it 'anticipates there being too many issues' do
       service = double
       allow(service).to receive(:execute) { raise IssueRebalancingService::TooManyIssues }
       expect(IssueRebalancingService).to receive(:new).with(issue).and_return(service)
-      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(IssueRebalancingService::TooManyIssues, include(issue_id: issue.id))
+      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(IssueRebalancingService::TooManyIssues, include(project_id: issue.project_id))
 
-      described_class.new.perform(issue.id)
+      described_class.new.perform(nil, issue.project_id)
+    end
+
+    it 'takes no action if the value is nil' do
+      expect(IssueRebalancingService).not_to receive(:new)
+      expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
+
+      described_class.new.perform(nil, nil)
     end
   end
 end