Merge branch 'allow-selecting-all-queues-with-sidekiq-cluster' into 'master'

Allow selecting all queues with sidekiq-cluster

See merge request gitlab-org/gitlab!26594

changelogs/unreleased/allow-selecting-all-queues-with-sidekiq-cluster.yml

+---
+title: Allow selecting all queues with sidekiq-cluster
+merge_request: 26594
+author:
+type: added

doc/administration/operations/extra_sidekiq_processes.md

@@ -53,6 +53,20 @@ To start extra Sidekiq processes, you must enable `sidekiq-cluster`:
    ]
    ```
 
+   [In GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26594) and
+   later, the special queue name `*` means all queues. This starts two
+   processes, each handling all queues:
+
+   ```ruby
+   sidekiq_cluster['queue_groups'] = [
+     "*",
+     "*"
+   ]
+   ```
+
+   `*` cannot be combined with concrete queue names - `*, mailers` will
+   just handle the `mailers` queue.
+
 1. Save the file and reconfigure GitLab for the changes to take effect:
 
    ```shell
@@ -154,6 +168,10 @@ from highest to lowest precedence:
 The operator precedence for this syntax is fixed: it's not possible to make AND
 have higher precedence than OR.
 
+[In GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26594) and
+later, as with the standard queue group syntax above, a single `*` as the
+entire queue group selects all queues.
+
 ### Example queries
 
 In `/etc/gitlab/gitlab.rb`:
@@ -163,9 +181,11 @@ sidekiq_cluster['enable'] = true
 sidekiq_cluster['experimental_queue_selector'] = true
 sidekiq_cluster['queue_groups'] = [
   # Run all non-CPU-bound queues that are high urgency
-  'resource_boundary!=cpu&urgency=high,
+  'resource_boundary!=cpu&urgency=high',
   # Run all continuous integration and pages queues that are not high urgency
-  'feature_category=continuous_integration,pages&urgency!=high
+  'feature_category=continuous_integration,pages&urgency!=high',
+  # Run all queues
+  '*'
 ]
 ```
 

ee/lib/gitlab/sidekiq_cluster.rb

@@ -45,10 +45,6 @@ module Gitlab
       pids.each { |pid| signal(pid, signal) }
     end
 
-    def self.parse_queues(array)
-      array.map { |chunk| chunk.split(',') }
-    end
-
     # Starts Sidekiq workers for the pairs of processes.
     #
     # Example:

ee/lib/gitlab/sidekiq_cluster/cli.rb

@@ -42,24 +42,28 @@ module Gitlab
         all_queues = SidekiqConfig::CliMethods.all_queues(@rails_path)
         queue_names = SidekiqConfig::CliMethods.worker_queues(@rails_path)
 
-        queue_groups =
+        queue_groups = argv.map do |queues|
+          next queue_names if queues == '*'
+
+          # When using the experimental queue query syntax, we treat
+          # each queue group as a worker attribute query, and resolve
+          # the queues for the queue group using this query.
           if @experimental_queue_selector
-            # When using the experimental queue query syntax, we treat
-            # each queue group as a worker attribute query, and resolve
-            # the queues for the queue group using this query.
-            argv.map do |queues|
-              SidekiqConfig::CliMethods.query_workers(queues, all_queues)
-            end
+            SidekiqConfig::CliMethods.query_workers(queues, all_queues)
           else
-            SidekiqCluster.parse_queues(argv).map do |queues|
-              SidekiqConfig::CliMethods.expand_queues(queues, queue_names)
-            end
+            SidekiqConfig::CliMethods.expand_queues(queues.split(','), queue_names)
           end
+        end
 
         if @negate_queues
           queue_groups.map! { |queues| queue_names - queues }
         end
 
+        if queue_groups.all?(&:empty?)
+          raise CommandError,
+            'No queues found, you must select at least one queue'
+        end
+
         @logger.info("Starting cluster with #{queue_groups.length} processes")
 
         @processes = SidekiqCluster.start(

ee/spec/bin/sidekiq_cluster_spec.rb

@@ -5,21 +5,41 @@ require 'spec_helper'
 describe 'ee/bin/sidekiq-cluster' do
   using RSpec::Parameterized::TableSyntax
 
-  where(:args, :included, :excluded) do
-    %w[--negate cronjob] | '-qdefault,1' | '-qcronjob,1'
-    %w[--experimental-queue-selector resource_boundary=cpu] | '-qupdate_merge_requests,1' | '-qdefault,1'
+  context 'when selecting some queues and excluding others' do
+    where(:args, :included, :excluded) do
+      %w[--negate cronjob] | '-qdefault,1' | '-qcronjob,1'
+      %w[--experimental-queue-selector resource_boundary=cpu] | '-qupdate_merge_requests,1' | '-qdefault,1'
+    end
+
+    with_them do
+      it 'runs successfully', :aggregate_failures do
+        cmd = %w[ee/bin/sidekiq-cluster --dryrun] + args
+
+        output, status = Gitlab::Popen.popen(cmd, Rails.root.to_s)
+
+        expect(status).to be(0)
+        expect(output).to include('"bundle", "exec", "sidekiq"')
+        expect(output).to include(included)
+        expect(output).not_to include(excluded)
+      end
+    end
   end
 
-  with_them do
-    it 'runs successfully', :aggregate_failures do
-      cmd = %w[ee/bin/sidekiq-cluster --dryrun] + args
+  context 'when selecting all queues' do
+    [
+      %w[*],
+      %w[--experimental-queue-selector *]
+    ].each do |args|
+      it "runs successfully with `#{args}`", :aggregate_failures do
+        cmd = %w[ee/bin/sidekiq-cluster --dryrun] + args
 
-      output, status = Gitlab::Popen.popen(cmd, Rails.root.to_s)
+        output, status = Gitlab::Popen.popen(cmd, Rails.root.to_s)
 
-      expect(status).to be(0)
-      expect(output).to include('"bundle", "exec", "sidekiq"')
-      expect(output).to include(included)
-      expect(output).not_to include(excluded)
+        expect(status).to be(0)
+        expect(output).to include('"bundle", "exec", "sidekiq"')
+        expect(output).to include('-qdefault,1')
+        expect(output).to include('-qcronjob:update_all_mirrors,1')
+      end
     end
   end
 end

ee/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb

@@ -35,6 +35,16 @@ describe Gitlab::SidekiqCluster::CLI do
         cli.run(%w(foo))
       end
 
+      it 'allows the special * selector' do
+        expect(Gitlab::SidekiqCluster)
+          .to receive(:start).with(
+            [Gitlab::SidekiqConfig::CliMethods.worker_queues],
+            default_options
+          )
+
+        cli.run(%w(*))
+      end
+
       context 'with --negate flag' do
         it 'starts Sidekiq workers for all queues in all_queues.yml except the ones in argv' do
           expect(Gitlab::SidekiqConfig::CliMethods).to receive(:worker_queues).and_return(['baz'])
@@ -150,6 +160,23 @@ describe Gitlab::SidekiqCluster::CLI do
           cli.run(%w(--experimental-queue-selector feature_category=chatops&urgency=high resource_boundary=memory&feature_category=importers))
         end
 
+        it 'allows the special * selector' do
+          expect(Gitlab::SidekiqCluster)
+            .to receive(:start).with(
+              [Gitlab::SidekiqConfig::CliMethods.worker_queues],
+              default_options
+            )
+
+          cli.run(%w(--experimental-queue-selector *))
+        end
+
+        it 'errors when the selector matches no queues' do
+          expect(Gitlab::SidekiqCluster).not_to receive(:start)
+
+          expect { cli.run(%w(--experimental-queue-selector has_external_dependencies=true&has_external_dependencies=false)) }
+            .to raise_error(described_class::CommandError)
+        end
+
         it 'errors on an invalid query multiple queue groups correctly' do
           expect(Gitlab::SidekiqCluster).not_to receive(:start)
 

ee/spec/lib/gitlab/sidekiq_cluster_spec.rb

@@ -51,13 +51,6 @@ describe Gitlab::SidekiqCluster do
     end
   end
 
-  describe '.parse_queues' do
-    it 'returns an Array containing the parsed queues' do
-      expect(described_class.parse_queues(%w(foo bar,baz)))
-        .to eq([%w(foo), %w(bar baz)])
-    end
-  end
-
   describe '.start' do
     it 'starts Sidekiq with the given queues, environment and options' do
       expected_options = {