Rework queue selector operators

We now have `|` for OR, `&` for AND, and `,` for concatenating
sets. This is hopefully more familiar to programmers and SREs.

Additionally, update the documentation to clarify that `=` and `!=` work
on the sets to their right, so they're more like 'in' and 'not in' than
'equal to' and 'not equal to'.

doc/administration/operations/extra_sidekiq_processes.md

@@ -125,19 +125,18 @@ considered false.
 `queue_selector` supports the following operators, listed from highest
 to lowest precedence:
 
-- <code> </code>&nbsp;(space) - 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 queues that match either query.
-- `,` - the logical AND operator. For example, `query_a,query_b` (where
+- `|` - 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
+  queues that match either query.
+- `&` - the logical AND operator. For example, `query_a&query_b` (where
   `query_a` and `query_b` are queries made up of the other operators here) will
   only include queues that match both queries.
-- `!=` - the not equal to operator. For example,
-  `feature_category!=issue_tracking` excludes all queues from the
-  `issue_tracking` feature category.
-- `=` - the equal to operator. For example, `resource_boundary=cpu` includes all
+- `!=` - the NOT IN operator. For example, `feature_category!=issue_tracking`
+  excludes all queues from the `issue_tracking` feature category.
+- `=` - the IN operator. For example, `resource_boundary=cpu` includes all
   queues that are CPU bound.
-- `|` - the concatenate set operator. For example,
-  `feature_category=continuous_integration|pages` includes all queues from
+- `,` - the concatenate set operator. For example,
+  `feature_category=continuous_integration,pages` includes all queues from
   either the `continuous_integration` category or the `pages` category. This
   example is also possible using the OR operator, but allows greater brevity, as
   well as being lower precedence.
@@ -153,10 +152,10 @@ In `/etc/gitlab/gitlab.rb`:
 sidekiq_cluster['enable'] = true
 sidekiq_cluster['queue_selector'] = true
 sidekiq_cluster['queue_groups'] = [
-  # Run all non-CPU-bound, queues that are latency sensitive
-  'resource_boundary!=cpu,latency_sensitive=true',
+  # Run all non-CPU-bound queues that are latency sensitive
+  'resource_boundary!=cpu&latency_sensitive=true',
   # Run all continuous integration and pages queues that are not latency sensitive
-  'feature_category=continuous_integration|pages,latency_sensitive=false'
+  'feature_category=continuous_integration,pages&latency_sensitive=false'
 ]
 ```
 

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

@@ -88,27 +88,27 @@ describe Gitlab::SidekiqCluster::CLI do
               excluded_queues: %w(merge)
             },
             'memory- or CPU-bound queues' => {
-              query: 'resource_boundary=memory|cpu',
+              query: 'resource_boundary=memory,cpu',
               included_queues: %w(auto_merge:auto_merge_process project_export),
               excluded_queues: %w(merge)
             },
             'latency-sensitive CI queues' => {
-              query: 'feature_category=continuous_integration,latency_sensitive=true',
+              query: 'feature_category=continuous_integration&latency_sensitive=true',
               included_queues: %w(pipeline_cache:expire_job_cache pipeline_cache:expire_pipeline_cache),
               excluded_queues: %w(merge)
             },
             'CPU-bound latency-sensitive CI queues' => {
-              query: 'feature_category=continuous_integration,latency_sensitive=true,resource_boundary=cpu',
+              query: 'feature_category=continuous_integration&latency_sensitive=true&resource_boundary=cpu',
               included_queues: %w(pipeline_cache:expire_pipeline_cache),
               excluded_queues: %w(pipeline_cache:expire_job_cache merge)
             },
             'CPU-bound latency-sensitive non-CI queues' => {
-              query: 'feature_category!=continuous_integration,latency_sensitive=true,resource_boundary=cpu',
+              query: 'feature_category!=continuous_integration&latency_sensitive=true&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',
+              query: 'feature_category=continuous_integration|feature_category=source_code_management',
               included_queues: %w(pipeline_cache:expire_job_cache merge),
               excluded_queues: %w(mailers)
             }
@@ -147,7 +147,7 @@ describe Gitlab::SidekiqCluster::CLI do
                   .with([['chat_notification'], ['project_export']], default_options)
                   .and_return([])
 
-          cli.run(%w(--queue-selector feature_category=chatops,latency_sensitive=true resource_boundary=memory,feature_category=source_code_management))
+          cli.run(%w(--queue-selector feature_category=chatops&latency_sensitive=true resource_boundary=memory&feature_category=source_code_management))
         end
 
         it 'errors on an invalid query multiple queue groups correctly' do

lib/gitlab/sidekiq_config/cli_methods.rb

@@ -18,10 +18,10 @@ module Gitlab
         result
       end.freeze
 
-      QUERY_OR_OPERATOR = %r{\s+}.freeze
-      QUERY_AND_OPERATOR = ','
-      QUERY_CONCATENATE_OPERATOR = '|'
-      QUERY_TERM_REGEX = %r{^(\w+)(!?=)([\w|]+)}.freeze
+      QUERY_OR_OPERATOR = '|'
+      QUERY_AND_OPERATOR = '&'
+      QUERY_CONCATENATE_OPERATOR = ','
+      QUERY_TERM_REGEX = %r{^(\w+)(!?=)([\w#{QUERY_CONCATENATE_OPERATOR}]+)}.freeze
 
       QUERY_PREDICATES = {
         feature_category: :to_sym,
@@ -81,6 +81,16 @@ module Gitlab
       def query_string_to_lambda(query_string)
         or_clauses = query_string.split(QUERY_OR_OPERATOR).map do |and_clauses_string|
           and_clauses_predicates = and_clauses_string.split(QUERY_AND_OPERATOR).map do |term|
+            predicate_for_term(term)
+          end
+
+          lambda { |worker| and_clauses_predicates.all? { |predicate| predicate.call(worker) } }
+        end
+
+        lambda { |worker| or_clauses.any? { |predicate| predicate.call(worker) } }
+      end
+
+      def predicate_for_term(term)
         match = term.match(QUERY_TERM_REGEX)
 
         raise InvalidTerm.new("Invalid term: #{term}") unless match
@@ -90,12 +100,6 @@ module Gitlab
         predicate_for_op(op, predicate_factory(lhs, rhs.split(QUERY_CONCATENATE_OPERATOR)))
       end
 
-          lambda { |worker| and_clauses_predicates.all? { |predicate| predicate.call(worker) } }
-        end
-
-        lambda { |worker| or_clauses.any? { |predicate| predicate.call(worker) } }
-      end
-
       def predicate_for_op(op, predicate)
         case op
         when '='

spec/lib/gitlab/sidekiq_config/cli_methods_spec.rb

@@ -155,39 +155,39 @@ describe Gitlab::SidekiqConfig::CliMethods do
       where(:query, :selected_queues) do
         # feature_category
         'feature_category=category_a' | %w(a a_2)
-        'feature_category=category_a|category_c' | %w(a a_2 c)
-        'feature_category=category_a feature_category=category_c' | %w(a a_2 c)
+        'feature_category=category_a,category_c' | %w(a a_2 c)
+        'feature_category=category_a|feature_category=category_c' | %w(a a_2 c)
         'feature_category!=category_a' | %w(b c)
 
         # has_external_dependencies
         'has_external_dependencies=true' | %w(b)
         'has_external_dependencies=false' | %w(a a_2 c)
-        'has_external_dependencies=true|false' | %w(a a_2 b c)
-        'has_external_dependencies=true has_external_dependencies=false' | %w(a a_2 b c)
+        'has_external_dependencies=true,false' | %w(a a_2 b c)
+        'has_external_dependencies=true|has_external_dependencies=false' | %w(a a_2 b c)
         'has_external_dependencies!=true' | %w(a a_2 c)
 
         # latency_sensitive
         'latency_sensitive=true' | %w(a_2 b)
         'latency_sensitive=false' | %w(a c)
-        'latency_sensitive=true|false' | %w(a a_2 b c)
-        'latency_sensitive=true latency_sensitive=false' | %w(a a_2 b c)
+        'latency_sensitive=true,false' | %w(a a_2 b c)
+        'latency_sensitive=true|latency_sensitive=false' | %w(a a_2 b c)
         'latency_sensitive!=true' | %w(a c)
 
         # name
         'name=a' | %w(a)
-        'name=a|b' | %w(a b)
-        'name=a|a_2 name=b' | %w(a a_2 b)
-        'name!=a|a_2' | %w(b c)
+        'name=a,b' | %w(a b)
+        'name=a,a_2|name=b' | %w(a a_2 b)
+        'name!=a,a_2' | %w(b c)
 
         # resource_boundary
         'resource_boundary=memory' | %w(b c)
-        'resource_boundary=memory|cpu' | %w(a b c)
-        'resource_boundary=memory resource_boundary=cpu' | %w(a b c)
-        'resource_boundary!=memory|cpu' | %w(a_2)
+        'resource_boundary=memory,cpu' | %w(a b c)
+        'resource_boundary=memory|resource_boundary=cpu' | %w(a b c)
+        'resource_boundary!=memory,cpu' | %w(a_2)
 
         # combinations
-        'feature_category=category_a,latency_sensitive=true' | %w(a_2)
-        'feature_category=category_a,latency_sensitive=true feature_category=category_c' | %w(a_2 c)
+        'feature_category=category_a&latency_sensitive=true' | %w(a_2)
+        'feature_category=category_a&latency_sensitive=true|feature_category=category_c' | %w(a_2 c)
       end
 
       with_them do