Merge branch 'dm-dedicated-sidekiq-queues' into 'master'

Use a dedicated queue for each Sidekiq worker

See merge request gitlab-org/gitlab-ce!15882

7 files changed, 36 insertions, 22 deletions

diff --git a/spec/workers/concerns/application_worker_spec.rb b/spec/workers/concerns/application_worker_spec.rb
index 0145563e0ed..901d77178bc 100644
--- a/spec/workers/concerns/application_worker_spec.rb
+++ b/spec/workers/concerns/application_worker_spec.rb
@@ -17,6 +17,14 @@ describe ApplicationWorker do
     end
   end
 
+  describe '.queue_namespace' do
+    it 'sets the queue name based on the class name' do
+      worker.queue_namespace :some_namespace
+
+      expect(worker.queue).to eq('some_namespace:foo_bar_dummy')
+    end
+  end
+
   describe '.queue' do
     it 'returns the queue name' do
       worker.sidekiq_options queue: :some_queue
diff --git a/spec/workers/concerns/cluster_queue_spec.rb b/spec/workers/concerns/cluster_queue_spec.rb
index 5049886b55c..4118b9aa194 100644
--- a/spec/workers/concerns/cluster_queue_spec.rb
+++ b/spec/workers/concerns/cluster_queue_spec.rb
@@ -14,6 +14,6 @@ describe ClusterQueue do
 
   it 'sets a default pipelines queue automatically' do
     expect(worker.sidekiq_options['queue'])
-      .to eq :gcp_cluster
+      .to eq 'gcp_cluster:dummy'
   end
 end
diff --git a/spec/workers/concerns/cronjob_queue_spec.rb b/spec/workers/concerns/cronjob_queue_spec.rb
index 3ae1c5f54d8..c042a52f41f 100644
--- a/spec/workers/concerns/cronjob_queue_spec.rb
+++ b/spec/workers/concerns/cronjob_queue_spec.rb
@@ -13,7 +13,7 @@ describe CronjobQueue do
   end
 
   it 'sets the queue name of a worker' do
-    expect(worker.sidekiq_options['queue'].to_s).to eq('cronjob')
+    expect(worker.sidekiq_options['queue'].to_s).to eq('cronjob:dummy')
   end
 
   it 'disables retrying of failed jobs' do
diff --git a/spec/workers/concerns/gitlab/github_import/queue_spec.rb b/spec/workers/concerns/gitlab/github_import/queue_spec.rb
index 9c69ee32da1..a96f583aff7 100644
--- a/spec/workers/concerns/gitlab/github_import/queue_spec.rb
+++ b/spec/workers/concerns/gitlab/github_import/queue_spec.rb
@@ -11,6 +11,6 @@ describe Gitlab::GithubImport::Queue do
       include Gitlab::GithubImport::Queue
     end
 
-    expect(worker.sidekiq_options['queue']).to eq('github_importer')
+    expect(worker.sidekiq_options['queue']).to eq('github_importer:dummy')
   end
 end
diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb
index dd911760948..a312b307fce 100644
--- a/spec/workers/concerns/pipeline_queue_spec.rb
+++ b/spec/workers/concerns/pipeline_queue_spec.rb
@@ -14,15 +14,6 @@ describe PipelineQueue do
 
   it 'sets a default pipelines queue automatically' do
     expect(worker.sidekiq_options['queue'])
-      .to eq 'pipeline_default'
-  end
-
-  describe '.enqueue_in' do
-    it 'sets a custom sidekiq queue with prefix and group' do
-      worker.enqueue_in(group: :processing)
-
-      expect(worker.sidekiq_options['queue'])
-        .to eq 'pipeline_processing'
-    end
+      .to eq 'pipeline_default:dummy'
   end
 end
diff --git a/spec/workers/concerns/repository_check_queue_spec.rb b/spec/workers/concerns/repository_check_queue_spec.rb
index fdbbfcc90a5..d2eeecfc9a8 100644
--- a/spec/workers/concerns/repository_check_queue_spec.rb
+++ b/spec/workers/concerns/repository_check_queue_spec.rb
@@ -13,7 +13,7 @@ describe RepositoryCheckQueue do
   end
 
   it 'sets the queue name of a worker' do
-    expect(worker.sidekiq_options['queue'].to_s).to eq('repository_check')
+    expect(worker.sidekiq_options['queue'].to_s).to eq('repository_check:dummy')
   end
 
   it 'disables retrying of failed jobs' do
diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb
index 7ee0a51a263..9e3b99b3502 100644
--- a/spec/workers/every_sidekiq_worker_spec.rb
+++ b/spec/workers/every_sidekiq_worker_spec.rb
@@ -1,21 +1,36 @@
 require 'spec_helper'
 
 describe 'Every Sidekiq worker' do
-  it 'includes ApplicationWorker' do
-    expect(Gitlab::SidekiqConfig.workers).to all(include(ApplicationWorker))
-  end
-
   it 'does not use the default queue' do
     expect(Gitlab::SidekiqConfig.workers.map(&:queue)).not_to include('default')
   end
 
   it 'uses the cronjob queue when the worker runs as a cronjob' do
-    expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(eq('cronjob'))
+    expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(start_with('cronjob:'))
+  end
+
+  it 'has its queue in app/workers/all_queues.yml', :aggregate_failures do
+    file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set
+
+    worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set
+    worker_queues << ActionMailer::DeliveryJob.queue_name
+    worker_queues << 'default'
+
+    missing_from_file = worker_queues - file_worker_queues
+    expect(missing_from_file).to be_empty, "expected #{missing_from_file.to_a.inspect} to be in app/workers/all_queues.yml"
+
+    unncessarily_in_file = file_worker_queues - worker_queues
+    expect(unncessarily_in_file).to be_empty, "expected #{unncessarily_in_file.to_a.inspect} not to be in app/workers/all_queues.yml"
   end
 
-  it 'defines the queue in the Sidekiq configuration file' do
-    config_queue_names = Gitlab::SidekiqConfig.config_queues.to_set
+  it 'has its queue or namespace in config/sidekiq_queues.yml', :aggregate_failures do
+    config_queues = Gitlab::SidekiqConfig.config_queues.to_set
+
+    Gitlab::SidekiqConfig.workers.each do |worker|
+      queue = worker.queue
+      queue_namespace = queue.split(':').first
 
-    expect(Gitlab::SidekiqConfig.worker_queues).to all(be_in(config_queue_names))
+      expect(config_queues).to include(queue).or(include(queue_namespace))
+    end
   end
 end