Merge branch 'bvl-deduplicate-root-statistics-worker' into 'master'

Deduplicate the Namespaces::RootStatisticsWorker jobs

See merge request gitlab-org/gitlab!27601

app/workers/all_queues.yml

@@ -891,7 +891,7 @@
   :urgency: :low
   :resource_boundary: :unknown
   :weight: 1
-  :idempotent: 
+  :idempotent: true
 - :name: update_namespace_statistics:namespaces_schedule_aggregation
   :feature_category: :source_code_management
   :has_external_dependencies: 

app/workers/namespaces/root_statistics_worker.rb

 # frozen_string_literal: true
 
 module Namespaces
-  class RootStatisticsWorker # rubocop:disable Scalability/IdempotentWorker
+  class RootStatisticsWorker
     include ApplicationWorker
 
     queue_namespace :update_namespace_statistics
     feature_category :source_code_management
+    idempotent!
 
     def perform(namespace_id)
       namespace = Namespace.find(namespace_id)

lib/gitlab/sidekiq_middleware/duplicate_jobs.rb

@@ -5,8 +5,18 @@ require 'digest'
 module Gitlab
   module SidekiqMiddleware
     module DuplicateJobs
-      def self.drop_duplicates?
-        Feature.enabled?(:drop_duplicate_sidekiq_jobs)
+      DROPPABLE_QUEUES = Set.new([
+        Namespaces::RootStatisticsWorker.queue
+      ]).freeze
+
+      def self.drop_duplicates?(queue_name)
+        Feature.enabled?(:drop_duplicate_sidekiq_jobs) ||
+          drop_duplicates_for_queue?(queue_name)
+      end
+
+      private_class_method def self.drop_duplicates_for_queue?(queue_name)
+        DROPPABLE_QUEUES.include?(queue_name) &&
+          Feature.enabled?(:drop_duplicate_sidekiq_jobs_for_queue)
       end
     end
   end

lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb

@@ -67,7 +67,7 @@ module Gitlab
         end
 
         def droppable?
-          idempotent? && duplicate? && DuplicateJobs.drop_duplicates?
+          idempotent? && duplicate? && DuplicateJobs.drop_duplicates?(queue_name)
         end
 
         private

spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb

@@ -129,7 +129,8 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r
       before do
         allow(AuthorizedProjectsWorker).to receive(:idempotent?).and_return(idempotent)
         allow(duplicate_job).to receive(:duplicate?).and_return(duplicate)
-        stub_feature_flags(drop_duplicate_sidekiq_jobs: feature_enabled)
+        allow(Gitlab::SidekiqMiddleware::DuplicateJobs)
+          .to receive(:drop_duplicates?).with(queue).and_return(feature_enabled)
       end
 
       it 'is droppable when all conditions are met' do

spec/lib/gitlab/sidekiq_middleware/duplicate_jobs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::SidekiqMiddleware::DuplicateJobs do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '.drop_duplicates?' do
+    where(:global_feature_enabled, :selected_queue_enabled, :queue, :expected) do
+      true  | true  | described_class::DROPPABLE_QUEUES.first | true
+      true  | true  | "other_queue"                           | true
+      true  | false | described_class::DROPPABLE_QUEUES.first | true
+      true  | false | "other_queue"                           | true
+      false | true  | described_class::DROPPABLE_QUEUES.first | true
+      false | true  | "other_queue"                           | false
+      false | false | described_class::DROPPABLE_QUEUES.first | false
+      false | false | "other_queue"                           | false
+    end
+
+    with_them do
+      before do
+        stub_feature_flags(drop_duplicate_sidekiq_jobs: global_feature_enabled,
+                           drop_duplicate_sidekiq_jobs_for_queue: selected_queue_enabled)
+      end
+
+      it "allows dropping jobs when expected" do
+        expect(described_class.drop_duplicates?(queue)).to be(expected)
+      end
+    end
+  end
+end

spec/workers/namespaces/root_statistics_worker_spec.rb

@@ -74,4 +74,19 @@ describe Namespaces::RootStatisticsWorker, '#perform' do
       worker.perform(group.id)
     end
   end
+
+  it_behaves_like 'an idempotent worker' do
+    let(:job_args) { [group.id] }
+
+    it 'deletes one aggregation schedule' do
+      # Make sure the group and it's aggregation schedule are created before
+      # counting
+      group
+
+      expect { worker.perform(*job_args) }
+        .to change { Namespace::AggregationSchedule.count }.by(-1)
+      expect { worker.perform(*job_args) }
+        .not_to change { Namespace::AggregationSchedule.count }
+    end
+  end
 end