diff --git a/changelogs/unreleased/queue-selector-not-experimental.yml b/changelogs/unreleased/queue-selector-not-experimental.yml new file mode 100644 index 0000000000000000000000000000000000000000..bf0a9dd35712fb691d5e0b7a7d76cc2fcc4ed303 --- /dev/null +++ b/changelogs/unreleased/queue-selector-not-experimental.yml @@ -0,0 +1,5 @@ +--- +title: Mark Sidekiq queue selector as no longer experimental +merge_request: 46562 +author: +type: changed diff --git a/doc/administration/operations/extra_sidekiq_processes.md b/doc/administration/operations/extra_sidekiq_processes.md index 67e0c6bb178549f1aa84feb4d777f03451e83134..eed92d32ec9d05dca4da160c5db153c571dc3031 100644 --- a/doc/administration/operations/extra_sidekiq_processes.md +++ b/doc/administration/operations/extra_sidekiq_processes.md @@ -110,29 +110,21 @@ you list: sudo gitlab-ctl reconfigure ``` -## Queue selector (experimental) +## Queue selector > - [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/45) in [GitLab Starter](https://about.gitlab.com/pricing/) 12.8. > - [Sidekiq cluster including queue selector moved](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/181) to GitLab [Core](https://about.gitlab.com/pricing/#self-managed) in GitLab 12.10. +> - [Marked as supported](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/147) in GitLab [Core](https://about.gitlab.com/pricing/#self-managed) in GitLab 13.6. Renamed from `experimental_queue_selector` to `queue_selector`. -CAUTION: **Caution:** -As this is marked as **experimental**, it is subject to change at any -time, including **breaking backwards compatibility**. This is so that we -can react to changes we need for our GitLab.com deployment. We have a -tracking issue open to [remove the experimental -designation](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/147) -from this feature; please comment there if you are interested in using -this in your own deployment. - -In addition to selecting queues by name, as above, the -`experimental_queue_selector` option allows queue groups to be selected -in a more general way using the following components: +In addition to selecting queues by name, as above, the `queue_selector` +option allows queue groups to be selected in a more general way using +the following components: - Attributes that can be selected. - Operators used to construct a query. -When `experimental_queue_selector` is set, all `queue_groups` must be in -the queue selector syntax. +When `queue_selector` is set, all `queue_groups` must be in the queue +selector syntax. ### Available attributes @@ -140,8 +132,7 @@ the queue selector syntax. From the [list of all available attributes](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/all_queues.yml), -`experimental_queue_selector` allows selecting of queues by the -following attributes: +`queue_selector` allows selecting of queues by the following attributes: - `feature_category` - the [GitLab feature category](https://about.gitlab.com/direction/maturity/#category-maturity) the @@ -173,8 +164,8 @@ neither of those tags. ### Available operators -`experimental_queue_selector` supports the following operators, listed -from highest to lowest precedence: +`queue_selector` supports the following operators, listed from highest +to lowest precedence: - `|` - the logical OR operator. For example, `query_a|query_b` (where `query_a` and `query_b` are queries made up of the other operators here) will include @@ -205,7 +196,7 @@ In `/etc/gitlab/gitlab.rb`: ```ruby sidekiq['enable'] = true -sidekiq['experimental_queue_selector'] = true +sidekiq['queue_selector'] = true sidekiq['queue_groups'] = [ # Run all non-CPU-bound queues that are high urgency 'resource_boundary!=cpu&urgency=high', @@ -234,7 +225,7 @@ All of the aforementioned configuration options for `sidekiq` are available. By default, they will be configured as follows: ```ruby -sidekiq['experimental_queue_selector'] = false +sidekiq['queue_selector'] = false sidekiq['interval'] = nil sidekiq['max_concurrency'] = 50 sidekiq['min_concurrency'] = nil diff --git a/lib/gitlab/sidekiq_cluster/cli.rb b/lib/gitlab/sidekiq_cluster/cli.rb index 1e5d23a840579919cdf6e030be8d0385b1cc4455..e471517c50ad9e7b2f34ef33a2302a5068d892b5 100644 --- a/lib/gitlab/sidekiq_cluster/cli.rb +++ b/lib/gitlab/sidekiq_cluster/cli.rb @@ -47,16 +47,24 @@ def run(argv = ARGV) option_parser.parse!(argv) + # Remove with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 + if @queue_selector && @experimental_queue_selector + raise CommandError, + 'You cannot specify --queue-selector and --experimental-queue-selector together' + end + all_queues = SidekiqConfig::CliMethods.all_queues(@rails_path) queue_names = SidekiqConfig::CliMethods.worker_queues(@rails_path) 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 queue query syntax, we treat each queue group + # as a worker attribute query, and resolve the queues for the + # queue group using this query. + + # Simplify with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 + if @queue_selector || @experimental_queue_selector SidekiqConfig::CliMethods.query_workers(queues, all_queues) else SidekiqConfig::CliMethods.expand_queues(queues.split(','), queue_names) @@ -182,7 +190,12 @@ def option_parser @rails_path = path end - opt.on('--experimental-queue-selector', 'EXPERIMENTAL: Run workers based on the provided selector') do |experimental_queue_selector| + opt.on('--queue-selector', 'Run workers based on the provided selector') do |queue_selector| + @queue_selector = queue_selector + end + + # Remove with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 + opt.on('--experimental-queue-selector', 'DEPRECATED: use --queue-selector-instead') do |experimental_queue_selector| @experimental_queue_selector = experimental_queue_selector end diff --git a/spec/bin/sidekiq_cluster_spec.rb b/spec/bin/sidekiq_cluster_spec.rb index fc5e2ae861aafdcc099183dcad13ea3953d2ba07..503cc0999c5e186e5bea01df718354fcb0dbb5a2 100644 --- a/spec/bin/sidekiq_cluster_spec.rb +++ b/spec/bin/sidekiq_cluster_spec.rb @@ -9,6 +9,8 @@ context 'when selecting some queues and excluding others' do where(:args, :included, :excluded) do %w[--negate cronjob] | '-qdefault,1' | '-qcronjob,1' + %w[--queue-selector resource_boundary=cpu] | '-qupdate_merge_requests,1' | '-qdefault,1' + # Remove with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 %w[--experimental-queue-selector resource_boundary=cpu] | '-qupdate_merge_requests,1' | '-qdefault,1' end @@ -29,6 +31,8 @@ context 'when selecting all queues' do [ %w[*], + %w[--queue-selector *], + # Remove with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 %w[--experimental-queue-selector *] ].each do |args| it "runs successfully with `#{args}`", :aggregate_failures do diff --git a/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb b/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb index cf165d1770b5a0374d44fe9f18efb4c26f70cf32..74834fb9014f6ce19f92483fc38bb02479c4ae77 100644 --- a/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb +++ b/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb @@ -108,101 +108,114 @@ end end - context 'with --experimental-queue-selector' do - where do - { - 'memory-bound queues' => { - query: 'resource_boundary=memory', - included_queues: %w(project_export), - excluded_queues: %w(merge) - }, - 'memory- or CPU-bound queues' => { - query: 'resource_boundary=memory,cpu', - included_queues: %w(auto_merge:auto_merge_process project_export), - excluded_queues: %w(merge) - }, - 'high urgency CI queues' => { - query: 'feature_category=continuous_integration&urgency=high', - included_queues: %w(pipeline_cache:expire_job_cache pipeline_cache:expire_pipeline_cache), - excluded_queues: %w(merge) - }, - 'CPU-bound high urgency CI queues' => { - query: 'feature_category=continuous_integration&urgency=high&resource_boundary=cpu', - included_queues: %w(pipeline_cache:expire_pipeline_cache), - excluded_queues: %w(pipeline_cache:expire_job_cache merge) - }, - 'CPU-bound high urgency non-CI queues' => { - query: 'feature_category!=continuous_integration&urgency=high&resource_boundary=cpu', - included_queues: %w(new_issue), - excluded_queues: %w(pipeline_cache:expire_pipeline_cache) - }, - 'CI and SCM queues' => { - query: 'feature_category=continuous_integration|feature_category=source_code_management', - included_queues: %w(pipeline_cache:expire_job_cache merge), - excluded_queues: %w(mailers) - } - } + # Remove with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 + context 'with --queue-selector and --experimental-queue-selector' do + it 'errors' do + expect(Gitlab::SidekiqCluster).not_to receive(:start) + + expect { cli.run(%w(--queue-selector name=foo --experimental-queue-selector name=bar)) } + .to raise_error(described_class::CommandError) end + end - with_them do - it 'expands queues by attributes' do - expect(Gitlab::SidekiqCluster).to receive(:start) do |queues, opts| - expect(opts).to eq(default_options) - expect(queues.first).to include(*included_queues) - expect(queues.first).not_to include(*excluded_queues) + # Simplify with https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/646 + ['--queue-selector', '--experimental-queue-selector'].each do |flag| + context "with #{flag}" do + where do + { + 'memory-bound queues' => { + query: 'resource_boundary=memory', + included_queues: %w(project_export), + excluded_queues: %w(merge) + }, + 'memory- or CPU-bound queues' => { + query: 'resource_boundary=memory,cpu', + included_queues: %w(auto_merge:auto_merge_process project_export), + excluded_queues: %w(merge) + }, + 'high urgency CI queues' => { + query: 'feature_category=continuous_integration&urgency=high', + included_queues: %w(pipeline_cache:expire_job_cache pipeline_cache:expire_pipeline_cache), + excluded_queues: %w(merge) + }, + 'CPU-bound high urgency CI queues' => { + query: 'feature_category=continuous_integration&urgency=high&resource_boundary=cpu', + included_queues: %w(pipeline_cache:expire_pipeline_cache), + excluded_queues: %w(pipeline_cache:expire_job_cache merge) + }, + 'CPU-bound high urgency non-CI queues' => { + query: 'feature_category!=continuous_integration&urgency=high&resource_boundary=cpu', + included_queues: %w(new_issue), + excluded_queues: %w(pipeline_cache:expire_pipeline_cache) + }, + 'CI and SCM queues' => { + query: 'feature_category=continuous_integration|feature_category=source_code_management', + included_queues: %w(pipeline_cache:expire_job_cache merge), + excluded_queues: %w(mailers) + } + } + end + + with_them do + it 'expands queues by attributes' do + expect(Gitlab::SidekiqCluster).to receive(:start) do |queues, opts| + expect(opts).to eq(default_options) + expect(queues.first).to include(*included_queues) + expect(queues.first).not_to include(*excluded_queues) + + [] + end - [] + cli.run(%W(#{flag} #{query})) end - cli.run(%W(--experimental-queue-selector #{query})) - end + it 'works when negated' do + expect(Gitlab::SidekiqCluster).to receive(:start) do |queues, opts| + expect(opts).to eq(default_options) + expect(queues.first).not_to include(*included_queues) + expect(queues.first).to include(*excluded_queues) - it 'works when negated' do - expect(Gitlab::SidekiqCluster).to receive(:start) do |queues, opts| - expect(opts).to eq(default_options) - expect(queues.first).not_to include(*included_queues) - expect(queues.first).to include(*excluded_queues) + [] + end - [] + cli.run(%W(--negate #{flag} #{query})) end - - cli.run(%W(--negate --experimental-queue-selector #{query})) end - end - it 'expands multiple queue groups correctly' do - expect(Gitlab::SidekiqCluster) - .to receive(:start) - .with([['chat_notification'], ['project_export']], default_options) - .and_return([]) + it 'expands multiple queue groups correctly' do + expect(Gitlab::SidekiqCluster) + .to receive(:start) + .with([['chat_notification'], ['project_export']], default_options) + .and_return([]) - cli.run(%w(--experimental-queue-selector feature_category=chatops&has_external_dependencies=true resource_boundary=memory&feature_category=importers)) - end + cli.run(%W(#{flag} feature_category=chatops&has_external_dependencies=true resource_boundary=memory&feature_category=importers)) + end - it 'allows the special * selector' do - worker_queues = %w(foo bar baz) + it 'allows the special * selector' do + worker_queues = %w(foo bar baz) - expect(Gitlab::SidekiqConfig::CliMethods) - .to receive(:worker_queues).and_return(worker_queues) + expect(Gitlab::SidekiqConfig::CliMethods) + .to receive(:worker_queues).and_return(worker_queues) - expect(Gitlab::SidekiqCluster) - .to receive(:start).with([worker_queues], default_options) + expect(Gitlab::SidekiqCluster) + .to receive(:start).with([worker_queues], default_options) - cli.run(%w(--experimental-queue-selector *)) - end + cli.run(%W(#{flag} *)) + end - it 'errors when the selector matches no queues' do - expect(Gitlab::SidekiqCluster).not_to receive(:start) + 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 + expect { cli.run(%W(#{flag} 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) + it 'errors on an invalid query multiple queue groups correctly' do + expect(Gitlab::SidekiqCluster).not_to receive(:start) - expect { cli.run(%w(--experimental-queue-selector unknown_field=chatops)) } - .to raise_error(Gitlab::SidekiqConfig::CliMethods::QueryError) + expect { cli.run(%W(#{flag} unknown_field=chatops)) } + .to raise_error(Gitlab::SidekiqConfig::CliMethods::QueryError) + end end end end