Commit d1af35d2 authored by Sean McGivern's avatar Sean McGivern

Allow selecting all queues with sidekiq-cluster

Because it was designed for selecting specific queues, sidekiq-cluster
didn't have a convenient method to select all queues. As we want this to
become the default sidekiq entry point for all GitLab installations, and
the default is to handle all queues in a single process, we need that
option.

(This was possible before: using `--experimental-queue-selector`, you
could ask for something like `has_external_dependencies=true,false`, but
that's not obvious.)

Instead of making `*` a wildcard anywhere, which would be more general,
this commit just allows a single `*` in a queue group to represent all
queues. While this is less general, it's also simpler to implement, and
we can assume that YAGNI for the wildcards.
parent 6a739930
---
title: Allow selecting all queues with sidekiq-cluster
merge_request: 26594
author:
type: added
...@@ -53,6 +53,20 @@ To start extra Sidekiq processes, you must enable `sidekiq-cluster`: ...@@ -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: 1. Save the file and reconfigure GitLab for the changes to take effect:
```shell ```shell
...@@ -154,6 +168,10 @@ from highest to lowest precedence: ...@@ -154,6 +168,10 @@ from highest to lowest precedence:
The operator precedence for this syntax is fixed: it's not possible to make AND The operator precedence for this syntax is fixed: it's not possible to make AND
have higher precedence than OR. 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 ### Example queries
In `/etc/gitlab/gitlab.rb`: In `/etc/gitlab/gitlab.rb`:
...@@ -163,9 +181,11 @@ sidekiq_cluster['enable'] = true ...@@ -163,9 +181,11 @@ sidekiq_cluster['enable'] = true
sidekiq_cluster['experimental_queue_selector'] = true sidekiq_cluster['experimental_queue_selector'] = true
sidekiq_cluster['queue_groups'] = [ sidekiq_cluster['queue_groups'] = [
# Run all non-CPU-bound queues that are high urgency # 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 # 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
'*'
] ]
``` ```
......
...@@ -45,10 +45,6 @@ module Gitlab ...@@ -45,10 +45,6 @@ module Gitlab
pids.each { |pid| signal(pid, signal) } pids.each { |pid| signal(pid, signal) }
end end
def self.parse_queues(array)
array.map { |chunk| chunk.split(',') }
end
# Starts Sidekiq workers for the pairs of processes. # Starts Sidekiq workers for the pairs of processes.
# #
# Example: # Example:
......
...@@ -42,24 +42,28 @@ module Gitlab ...@@ -42,24 +42,28 @@ module Gitlab
all_queues = SidekiqConfig::CliMethods.all_queues(@rails_path) all_queues = SidekiqConfig::CliMethods.all_queues(@rails_path)
queue_names = SidekiqConfig::CliMethods.worker_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 if @experimental_queue_selector
# When using the experimental queue query syntax, we treat SidekiqConfig::CliMethods.query_workers(queues, all_queues)
# 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
else else
SidekiqCluster.parse_queues(argv).map do |queues| SidekiqConfig::CliMethods.expand_queues(queues.split(','), queue_names)
SidekiqConfig::CliMethods.expand_queues(queues, queue_names)
end
end end
end
if @negate_queues if @negate_queues
queue_groups.map! { |queues| queue_names - queues } queue_groups.map! { |queues| queue_names - queues }
end 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") @logger.info("Starting cluster with #{queue_groups.length} processes")
@processes = SidekiqCluster.start( @processes = SidekiqCluster.start(
......
...@@ -5,21 +5,41 @@ require 'spec_helper' ...@@ -5,21 +5,41 @@ require 'spec_helper'
describe 'ee/bin/sidekiq-cluster' do describe 'ee/bin/sidekiq-cluster' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:args, :included, :excluded) do context 'when selecting some queues and excluding others' do
%w[--negate cronjob] | '-qdefault,1' | '-qcronjob,1' where(:args, :included, :excluded) do
%w[--experimental-queue-selector resource_boundary=cpu] | '-qupdate_merge_requests,1' | '-qdefault,1' %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 end
with_them do context 'when selecting all queues' do
it 'runs successfully', :aggregate_failures do [
cmd = %w[ee/bin/sidekiq-cluster --dryrun] + args %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(status).to be(0)
expect(output).to include('"bundle", "exec", "sidekiq"') expect(output).to include('"bundle", "exec", "sidekiq"')
expect(output).to include(included) expect(output).to include('-qdefault,1')
expect(output).not_to include(excluded) expect(output).to include('-qcronjob:update_all_mirrors,1')
end
end end
end end
end end
...@@ -35,6 +35,16 @@ describe Gitlab::SidekiqCluster::CLI do ...@@ -35,6 +35,16 @@ describe Gitlab::SidekiqCluster::CLI do
cli.run(%w(foo)) cli.run(%w(foo))
end 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 context 'with --negate flag' do
it 'starts Sidekiq workers for all queues in all_queues.yml except the ones in argv' 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']) expect(Gitlab::SidekiqConfig::CliMethods).to receive(:worker_queues).and_return(['baz'])
...@@ -150,6 +160,23 @@ describe Gitlab::SidekiqCluster::CLI do ...@@ -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)) cli.run(%w(--experimental-queue-selector feature_category=chatops&urgency=high resource_boundary=memory&feature_category=importers))
end 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 it 'errors on an invalid query multiple queue groups correctly' do
expect(Gitlab::SidekiqCluster).not_to receive(:start) expect(Gitlab::SidekiqCluster).not_to receive(:start)
......
...@@ -51,13 +51,6 @@ describe Gitlab::SidekiqCluster do ...@@ -51,13 +51,6 @@ describe Gitlab::SidekiqCluster do
end end
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 describe '.start' do
it 'starts Sidekiq with the given queues, environment and options' do it 'starts Sidekiq with the given queues, environment and options' do
expected_options = { expected_options = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment