Merge branch '321982-enqueue-jobs-to-new-issues-workers-only' into 'master'

Enqueue new issues workers vs old ones

See merge request gitlab-org/gitlab!73423

app/models/issue.rb

@@ -605,7 +605,7 @@ class Issue < ApplicationRecord
 
   def could_not_move(exception)
     # Symptom of running out of space - schedule rebalancing
-    IssueRebalancingWorker.perform_async(nil, *project.self_or_root_group_ids)
+    Issues::RebalancingWorker.perform_async(nil, *project.self_or_root_group_ids)
   end
 end
 

app/services/issues/base_service.rb

@@ -30,7 +30,7 @@ module Issues
       gates = [issue.project, issue.project.group].compact
       return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) }
 
-      IssueRebalancingWorker.perform_async(nil, *issue.project.self_or_root_group_ids)
+      Issues::RebalancingWorker.perform_async(nil, *issue.project.self_or_root_group_ids)
     end
 
     private

app/services/issues/create_service.rb

@@ -41,7 +41,7 @@ module Issues
       user = current_user
       issue.run_after_commit do
         NewIssueWorker.perform_async(issue.id, user.id)
-        IssuePlacementWorker.perform_async(nil, issue.project_id)
+        Issues::PlacementWorker.perform_async(nil, issue.project_id)
         Namespaces::OnboardingIssueCreatedWorker.perform_async(issue.namespace.id)
       end
     end

app/workers/issue_placement_worker.rb

 # frozen_string_literal: true
 
+# DEPRECATED. Will be removed in 14.7 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72803
+# Please use Issues::PlacementWorker instead
+#
 # todo: remove this worker and it's queue definition from all_queues after Issues::PlacementWorker is deployed
 # We want to keep it for one release in case some jobs are already scheduled in the old queue so we need the worker
 # to be available to finish those. All new jobs will be queued into the new queue.
@@ -43,10 +46,10 @@ class IssuePlacementWorker
 
     Issue.move_nulls_to_end(to_place)
     Issues::BaseService.new(project: nil).rebalance_if_needed(to_place.max_by(&:relative_position))
-    IssuePlacementWorker.perform_async(nil, leftover.project_id) if leftover.present?
+    Issues::PlacementWorker.perform_async(nil, leftover.project_id) if leftover.present?
   rescue RelativePositioning::NoSpaceLeft => e
     Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id, project_id: project_id)
-    IssueRebalancingWorker.perform_async(nil, *root_namespace_id_to_rebalance(issue, project_id))
+    Issues::RebalancingWorker.perform_async(nil, *root_namespace_id_to_rebalance(issue, project_id))
   end
 
   def find_issue(issue_id, project_id)

app/workers/issue_rebalancing_worker.rb

 # frozen_string_literal: true
 
+# DEPRECATED. Will be removed in 14.7 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72803
+# Please use Issues::RebalancingWorker instead
+#
 # todo: remove this worker and it's queue definition from all_queues after Issue::RebalancingWorker is released.
 # We want to keep it for one release in case some jobs are already scheduled in the old queue so we need the worker
 # to be available to finish those. All new jobs will be queued into the new queue.

app/workers/issues/rebalancing_worker.rb

@@ -17,6 +17,7 @@ module Issues
       # we need to have exactly one of the project_id and root_namespace_id params be non-nil
       raise ArgumentError, "Expected only one of the params project_id: #{project_id} and root_namespace_id: #{root_namespace_id}" if project_id && root_namespace_id
       return if project_id.nil? && root_namespace_id.nil?
+      return if ::Gitlab::Issues::Rebalancing::State.rebalance_recently_finished?(project_id, root_namespace_id)
 
       # pull the projects collection to be rebalanced either the project if namespace is not a group(i.e. user namesapce)
       # or the root namespace, this also makes the worker backward compatible with previous version where a project_id was

app/workers/issues/reschedule_stuck_issue_rebalances_worker.rb

@@ -20,13 +20,13 @@ module Issues
       namespaces = Namespace.id_in(namespace_ids)
       projects = Project.id_in(project_ids)
 
-      IssueRebalancingWorker.bulk_perform_async_with_contexts(
+      Issues::RebalancingWorker.bulk_perform_async_with_contexts(
         namespaces,
         arguments_proc: -> (namespace) { [nil, nil, namespace.id] },
         context_proc: -> (namespace) { { namespace: namespace } }
       )
 
-      IssueRebalancingWorker.bulk_perform_async_with_contexts(
+      Issues::RebalancingWorker.bulk_perform_async_with_contexts(
         projects,
         arguments_proc: -> (project) { [nil, project.id, nil] },
         context_proc: -> (project) { { project: project } }

spec/models/issue_spec.rb

@@ -1462,7 +1462,7 @@ RSpec.describe Issue do
       it 'schedules rebalancing if there is no space left' do
         lhs = build_stubbed(:issue, relative_position: 99, project: project)
         to_move = build(:issue, project: project)
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project_id, namespace_id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, project_id, namespace_id)
 
         expect { to_move.move_between(lhs, issue) }.to raise_error(RelativePositioning::NoSpaceLeft)
       end

spec/services/issues/create_service_spec.rb

@@ -154,7 +154,7 @@ RSpec.describe Issues::CreateService do
       end
 
       it 'moves the issue to the end, in an asynchronous worker' do
-        expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer)
+        expect(Issues::PlacementWorker).to receive(:perform_async).with(be_nil, Integer)
 
         described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
       end

spec/services/issues/update_service_spec.rb

@@ -319,7 +319,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).not_to receive(:perform_async)
+        expect(Issues::RebalancingWorker).not_to receive(:perform_async)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -335,7 +335,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -349,7 +349,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
@@ -363,7 +363,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.root_namespace.id)
 
         update_issue(opts)
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)

spec/workers/issue_placement_worker_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe IssuePlacementWorker do
       it 'schedules rebalancing if needed' do
         issue_a.update!(relative_position: RelativePositioning::MAX_POSITION)
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
 
         run_worker
       end
@@ -52,7 +52,7 @@ RSpec.describe IssuePlacementWorker do
                              .with(have_attributes(count: described_class::QUERY_LIMIT))
                              .and_call_original
 
-          expect(described_class).to receive(:perform_async).with(nil, project.id)
+          expect(Issues::PlacementWorker).to receive(:perform_async).with(nil, project.id)
 
           run_worker
 
@@ -101,7 +101,7 @@ RSpec.describe IssuePlacementWorker do
       it 'anticipates the failure to place the issues, and schedules rebalancing' do
         allow(Issue).to receive(:move_nulls_to_end) { raise RelativePositioning::NoSpaceLeft }
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
         expect(Gitlab::ErrorTracking)
           .to receive(:log_exception)
           .with(RelativePositioning::NoSpaceLeft, worker_arguments)

spec/workers/issues/placement_worker_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe Issues::PlacementWorker do
       it 'schedules rebalancing if needed' do
         issue_a.update!(relative_position: RelativePositioning::MAX_POSITION)
 
-        expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
+        expect(Issues::RebalancingWorker).to receive(:perform_async).with(nil, nil, project.group.id)
 
         run_worker
       end

spec/workers/issues/rebalancing_worker_spec.rb

@@ -35,6 +35,20 @@ RSpec.describe Issues::RebalancingWorker do
 
         described_class.new.perform # all arguments are nil
       end
+
+      it 'does not schedule a new rebalance if it finished under 1h ago' do
+        container_type = arguments.second.present? ? ::Gitlab::Issues::Rebalancing::State::PROJECT : ::Gitlab::Issues::Rebalancing::State::NAMESPACE
+        container_id = arguments.second || arguments.third
+
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.set(::Gitlab::Issues::Rebalancing::State.send(:recently_finished_key, container_type, container_id), true)
+        end
+
+        expect(Issues::RelativePositionRebalancingService).not_to receive(:new)
+        expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
+
+        described_class.new.perform(*arguments)
+      end
     end
 
     shared_examples 'safely handles non-existent ids' do

spec/workers/issues/reschedule_stuck_issue_rebalances_worker_spec.rb

@@ -10,15 +10,15 @@ RSpec.describe Issues::RescheduleStuckIssueRebalancesWorker, :clean_gitlab_redis
 
   describe '#perform' do
     it 'does not schedule a rebalance' do
-      expect(IssueRebalancingWorker).not_to receive(:perform_async)
+      expect(Issues::RebalancingWorker).not_to receive(:perform_async)
 
       worker.perform
     end
 
     it 'schedules a rebalance in case there are any rebalances started' do
       expect(::Gitlab::Issues::Rebalancing::State).to receive(:fetch_rebalancing_groups_and_projects).and_return([[group.id], [project.id]])
-      expect(IssueRebalancingWorker).to receive(:bulk_perform_async).with([[nil, nil, group.id]]).once
-      expect(IssueRebalancingWorker).to receive(:bulk_perform_async).with([[nil, project.id, nil]]).once
+      expect(Issues::RebalancingWorker).to receive(:bulk_perform_async).with([[nil, nil, group.id]]).once
+      expect(Issues::RebalancingWorker).to receive(:bulk_perform_async).with([[nil, project.id, nil]]).once
 
       worker.perform
     end