Commit e2e7760a authored by Robert Speicher's avatar Robert Speicher

Merge branch 'rename-latency-sensitive-to-urgency' into 'master'

Rename latency_sensitive -> urgency

See merge request gitlab-org/gitlab!26271
parents 9d2f6887 74ea5a99
This diff is collapsed.
......@@ -5,7 +5,7 @@ class AuthorizedProjectsWorker # rubocop:disable Scalability/IdempotentWorker
prepend WaitableWorker
feature_category :authentication_and_authorization
latency_sensitive_worker!
urgency :high
weight 2
# This is a workaround for a Ruby 2.3.7 bug. rspec-mocks cannot restore the
......
......@@ -5,7 +5,7 @@ class BuildFinishedWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_processing
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -6,7 +6,7 @@ class BuildHooksWorker # rubocop:disable Scalability/IdempotentWorker
queue_namespace :pipeline_hooks
feature_category :continuous_integration
latency_sensitive_worker!
urgency :high
# rubocop: disable CodeReuse/ActiveRecord
def perform(build_id)
......
......@@ -6,7 +6,7 @@ class BuildQueueWorker # rubocop:disable Scalability/IdempotentWorker
queue_namespace :pipeline_processing
feature_category :continuous_integration
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -5,7 +5,7 @@ class BuildSuccessWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_processing
latency_sensitive_worker!
urgency :high
# rubocop: disable CodeReuse/ActiveRecord
def perform(build_id)
......
......@@ -7,7 +7,7 @@ class ChatNotificationWorker # rubocop:disable Scalability/IdempotentWorker
sidekiq_options retry: false
feature_category :chatops
latency_sensitive_worker!
urgency :high
weight 2
# TODO: break this into multiple jobs
......
......@@ -5,7 +5,7 @@ module Ci
include ::ApplicationWorker
include ::PipelineQueue
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(pipeline_id)
......
......@@ -4,9 +4,12 @@ module WorkerAttributes
extend ActiveSupport::Concern
# Resource boundaries that workers can declare through the
# `worker_resource_boundary` attribute
# `resource_boundary` attribute
VALID_RESOURCE_BOUNDARIES = [:memory, :cpu, :unknown].freeze
# Urgencies that workers can declare through the `urgencies` attribute
VALID_URGENCIES = [:high, :default, :none].freeze
NAMESPACE_WEIGHTS = {
auto_devops: 2,
auto_merge: 3,
......@@ -47,21 +50,22 @@ module WorkerAttributes
get_worker_attribute(:feature_category) == :not_owned
end
# This should be set for jobs that need to be run immediately, or, if
# they are delayed, risk creating inconsistencies in the application
# that could being perceived by the user as incorrect behavior
# (ie, a bug)
# See doc/development/sidekiq_style_guide.md#Latency-Sensitive-Jobs
# This should be set to :high for jobs that need to be run
# immediately, or, if they are delayed, risk creating
# inconsistencies in the application that could being perceived by
# the user as incorrect behavior (ie, a bug)
#
# See
# doc/development/sidekiq_style_guide.md#urgency
# for details
def latency_sensitive_worker!
worker_attributes[:latency_sensitive] = true
def urgency(urgency)
raise "Invalid urgency: #{urgency}" unless VALID_URGENCIES.include?(urgency)
worker_attributes[:urgency] = urgency
end
# Returns a truthy value if the worker is latency sensitive.
# See doc/development/sidekiq_style_guide.md#Latency-Sensitive-Jobs
# for details
def latency_sensitive_worker?
worker_attributes[:latency_sensitive]
def get_urgency
worker_attributes[:urgency] || :default
end
# Set this attribute on a job when it will call to services outside of the
......
......@@ -6,7 +6,7 @@ class CreatePipelineWorker # rubocop:disable Scalability/IdempotentWorker
queue_namespace :pipeline_creation
feature_category :continuous_integration
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(project_id, user_id, ref, source, params = {})
......
......@@ -4,7 +4,7 @@ class EmailReceiverWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :issue_tracking
latency_sensitive_worker!
urgency :high
weight 2
def perform(raw)
......
......@@ -6,7 +6,7 @@ class EmailsOnPushWorker # rubocop:disable Scalability/IdempotentWorker
attr_reader :email, :skip_premailer
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 2
......
......@@ -5,7 +5,7 @@ class ExpireJobCacheWorker
include PipelineQueue
queue_namespace :pipeline_cache
latency_sensitive_worker!
urgency :high
idempotent!
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -5,7 +5,7 @@ class ExpirePipelineCacheWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_cache
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -5,7 +5,7 @@ class GitlabShellWorker # rubocop:disable Scalability/IdempotentWorker
include Gitlab::ShellAdapter
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
weight 2
def perform(action, *arg)
......
......@@ -4,7 +4,7 @@ class MergeWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
weight 5
def perform(merge_request_id, current_user_id, params)
......
......@@ -5,7 +5,7 @@ class NewIssueWorker # rubocop:disable Scalability/IdempotentWorker
include NewIssuable
feature_category :issue_tracking
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 2
......
......@@ -5,7 +5,7 @@ class NewMergeRequestWorker # rubocop:disable Scalability/IdempotentWorker
include NewIssuable
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 2
......
......@@ -4,7 +4,7 @@ class NewNoteWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :issue_tracking
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 2
......
......@@ -5,7 +5,7 @@ class PipelineHooksWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_hooks
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -4,7 +4,7 @@ class PipelineMetricsWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
include PipelineQueue
latency_sensitive_worker!
urgency :high
# rubocop: disable CodeReuse/ActiveRecord
def perform(pipeline_id)
......
......@@ -4,7 +4,7 @@ class PipelineNotificationWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
include PipelineQueue
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -6,7 +6,7 @@ class PipelineProcessWorker # rubocop:disable Scalability/IdempotentWorker
queue_namespace :pipeline_processing
feature_category :continuous_integration
latency_sensitive_worker!
urgency :high
# rubocop: disable CodeReuse/ActiveRecord
def perform(pipeline_id, build_ids = nil)
......
......@@ -5,7 +5,7 @@ class PipelineSuccessWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_processing
latency_sensitive_worker!
urgency :high
def perform(pipeline_id)
# no-op
......
......@@ -4,7 +4,7 @@ class PipelineUpdateCiRefStatusWorker # rubocop:disable Scalability/IdempotentWo
include ApplicationWorker
include PipelineQueue
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(pipeline_id)
......
......@@ -5,7 +5,7 @@ class PipelineUpdateWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_processing
latency_sensitive_worker!
urgency :high
def perform(pipeline_id)
Ci::Pipeline.find_by_id(pipeline_id)&.update_legacy_status
......
......@@ -4,7 +4,7 @@ class PostReceive # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 5
......
......@@ -11,7 +11,7 @@ class ProcessCommitWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
weight 3
# project_id - The ID of the project this commit belongs to.
......
......@@ -4,7 +4,7 @@
class ProjectCacheWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
latency_sensitive_worker!
urgency :high
LEASE_TIMEOUT = 15.minutes.to_i
......
......@@ -6,11 +6,11 @@ class ReactiveCachingWorker # rubocop:disable Scalability/IdempotentWorker
feature_category_not_owned!
# TODO: The reactive caching worker should be split into
# two different workers, one for latency_sensitive jobs without external dependencies
# and another worker without latency_sensitivity, but with external dependencies
# two different workers, one for high urgency jobs without external dependencies
# and another worker without high urgency, but with external dependencies
# https://gitlab.com/gitlab-com/gl-infra/scalability/issues/34
# This worker should also have `worker_has_external_dependencies!` enabled
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(class_name, id, *args)
......
......@@ -5,7 +5,7 @@ class StageUpdateWorker # rubocop:disable Scalability/IdempotentWorker
include PipelineQueue
queue_namespace :pipeline_processing
latency_sensitive_worker!
urgency :high
def perform(stage_id)
Ci::Stage.find_by_id(stage_id)&.update_legacy_status
......
......@@ -6,7 +6,7 @@ class UpdateHeadPipelineForMergeRequestWorker # rubocop:disable Scalability/Idem
queue_namespace :pipeline_processing
feature_category :continuous_integration
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(merge_request_id)
......
......@@ -4,7 +4,7 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
feature_category :source_code_management
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
weight 3
......
......@@ -115,10 +115,10 @@ following attributes:
`source_code_management` category.
- `has_external_dependencies` - whether or not the queue connects to external
services. For example, all importers have this set to `true`.
- `latency_sensitive` - whether or not the queue is particularly sensitive to
latency, which also means that its jobs should run quickly. For example, the
`authorized_projects` queue is used to refresh user permissions, and is
latency sensitive.
- `urgency` - how important it is that this queue's jobs run
quickly. Can be `high`, `default`, or `none`. For example, the
`authorized_projects` queue is used to refresh user permissions, and
is high urgency.
- `name` - the queue name. The other attributes are typically more useful as
they are more general, but this is available in case a particular queue needs
to be selected.
......@@ -126,9 +126,9 @@ following attributes:
`unknown`. For example, the `project_export` queue is memory bound as it has
to load data in memory before saving it for export.
Both `has_external_dependencies` and `latency_sensitive` are boolean attributes:
only the exact string `true` is considered true, and everything else is
considered false.
`has_external_dependencies` is a boolean attribute: only the exact
string `true` is considered true, and everything else is considered
false.
### Available operators
......@@ -162,10 +162,10 @@ In `/etc/gitlab/gitlab.rb`:
sidekiq_cluster['enable'] = true
sidekiq_cluster['experimental_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 continuous integration and pages queues that are not latency sensitive
'feature_category=continuous_integration,pages&latency_sensitive=false'
# Run all non-CPU-bound queues that are high urgency
'resource_boundary!=cpu&urgency=high,
# Run all continuous integration and pages queues that are not high urgency
'feature_category=continuous_integration,pages&urgency!=high
]
```
......
......@@ -121,7 +121,30 @@ NOTE: **Note:**
Note that a cop will fail if the worker class is not marked as idempotent.
Consider skipping the cop if you're not confident your job can safely run multiple times.
## Latency Sensitive Jobs
## Job urgency
Jobs can have an `urgency` attribute set, which can be `:high`,
`:default`, or `:none`. These have the below targets:
| **Urgency** | **Queue Scheduling Target** | **Execution Latency Requirement** |
|-------------|-----------------------------|------------------------------------|
| `:high` | 100 milliseconds | p50 of 1 second, p99 of 10 seconds |
| `:default` | 1 minute | Maximum run time of 1 hour |
| `:none` | None | Maximum run time of 1 hour |
To set a job's urgency, use the `urgency` class method:
```ruby
class HighUrgencyWorker
include ApplicationWorker
urgency :high
# ...
end
```
### Latency sensitive jobs
If a large number of background jobs get scheduled at once, queueing of jobs may
occur while jobs wait for a worker node to be become available. This is normal
......@@ -140,7 +163,7 @@ of these jobs include:
When these jobs are delayed, the user may perceive the delay as a bug: for
example, they may push a branch and then attempt to create a merge request for
that branch, but be told in the UI that the branch does not exist. We deem these
jobs to be `latency_sensitive`.
jobs to be `urgency :high`.
Extra effort is made to ensure that these jobs are started within a very short
period of time after being scheduled. However, in order to ensure throughput,
......@@ -150,31 +173,11 @@ these jobs also have very strict execution duration requirements:
1. 99% of jobs should complete within 10 seconds.
If a worker cannot meet these expectations, then it cannot be treated as a
`latency_sensitive` worker: consider redesigning the worker, or splitting the
work between two different workers, one with `latency_sensitive` code that
executes quickly, and the other with non-`latency_sensitive`, which has no
`urgency :high` worker: consider redesigning the worker, or splitting the
work between two different workers, one with `urgency :high` code that
executes quickly, and the other with `urgency :default`, which has no
execution latency requirements (but also has lower scheduling targets).
This can be summed up in the following table:
| **Latency Sensitivity** | **Queue Scheduling Target** | **Execution Latency Requirement** |
|-------------------------|-----------------------------|-------------------------------------|
| Not `latency_sensitive` | 1 minute | Maximum run time of 1 hour |
| `latency_sensitive` | 100 milliseconds | p50 of 1 second, p99 of 10 seconds |
To mark a worker as being `latency_sensitive`, use the
`latency_sensitive_worker!` attribute, as shown in this example:
```ruby
class LatencySensitiveWorker
include ApplicationWorker
latency_sensitive_worker!
# ...
end
```
## Jobs with External Dependencies
Most background jobs in the GitLab application communicate with other GitLab
......@@ -194,7 +197,7 @@ the background processing cluster in several ways:
therefore we cannot guarantee the execution latencies on these jobs. Since we
cannot guarantee execution latency, we cannot ensure throughput and
therefore, in high-traffic environments, we need to ensure that jobs with
external dependencies are separated from `latency_sensitive` jobs, to ensure
external dependencies are separated from high urgency jobs, to ensure
throughput on those queues.
1. Errors in jobs with external dependencies have higher alerting thresholds as
there is a likelihood that the cause of the error is external.
......@@ -212,7 +215,7 @@ class ExternalDependencyWorker
end
```
NOTE: **Note:** Note that a job cannot be both latency sensitive and have
NOTE: **Note:** Note that a job cannot be both high urgency and have
external dependencies.
## CPU-bound and Memory-bound Workers
......@@ -246,14 +249,14 @@ bespoke low concurrency, high memory fleet.
Note that memory-bound workers create heavy GC workloads, with pauses of
10-50ms. This will have an impact on the latency requirements for the
worker. For this reason, `memory` bound, `latency_sensitive` jobs are not
worker. For this reason, `memory` bound, `urgency :high` jobs are not
permitted and will fail CI. In general, `memory` bound workers are
discouraged, and alternative approaches to processing the work should be
considered.
If a worker needs large amounts of both memory and CPU time, it should be marked as
memory-bound, due to the above restrction on latency-sensitive memory-bound
workers.
If a worker needs large amounts of both memory and CPU time, it should
be marked as memory-bound, due to the above restrction on high urgency
memory-bound workers.
## Declaring a Job as CPU-bound
......
This diff is collapsed.
......@@ -6,7 +6,7 @@ class SyncSecurityReportsToReportApprovalRulesWorker # rubocop:disable Scalabili
include ApplicationWorker
include SecurityScansQueue
latency_sensitive_worker!
urgency :high
worker_resource_boundary :cpu
def perform(pipeline_id)
......
......@@ -92,18 +92,18 @@ describe Gitlab::SidekiqCluster::CLI do
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',
'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 latency-sensitive CI queues' => {
query: 'feature_category=continuous_integration&latency_sensitive=true&resource_boundary=cpu',
'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 latency-sensitive non-CI queues' => {
query: 'feature_category!=continuous_integration&latency_sensitive=true&resource_boundary=cpu',
'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)
},
......@@ -147,7 +147,7 @@ describe Gitlab::SidekiqCluster::CLI do
.with([['chat_notification'], ['project_export']], default_options)
.and_return([])
cli.run(%w(--experimental-queue-selector feature_category=chatops&latency_sensitive=true resource_boundary=memory&feature_category=importers))
cli.run(%w(--experimental-queue-selector feature_category=chatops&urgency=high resource_boundary=memory&feature_category=importers))
end
it 'errors on an invalid query multiple queue groups correctly' do
......
......@@ -26,9 +26,9 @@ module Gitlab
QUERY_PREDICATES = {
feature_category: :to_sym,
has_external_dependencies: lambda { |value| value == 'true' },
latency_sensitive: lambda { |value| value == 'true' },
name: :to_s,
resource_boundary: :to_sym
resource_boundary: :to_sym,
urgency: :to_sym
}.freeze
QueryError = Class.new(StandardError)
......
......@@ -9,7 +9,7 @@ module Gitlab
ATTRIBUTE_METHODS = {
feature_category: :get_feature_category,
has_external_dependencies: :worker_has_external_dependencies?,
latency_sensitive: :latency_sensitive_worker?,
urgency: :get_urgency,
resource_boundary: :get_worker_resource_boundary,
idempotent: :idempotent?,
weight: :get_weight
......
......@@ -7,8 +7,8 @@ module Gitlab
attr_reader :klass
delegate :feature_category_not_owned?, :get_feature_category,
:get_weight, :get_worker_resource_boundary, :idempotent?,
:latency_sensitive_worker?, :queue, :queue_namespace,
:get_urgency, :get_weight, :get_worker_resource_boundary,
:idempotent?, :queue, :queue_namespace,
:worker_has_external_dependencies?,
to: :klass
......@@ -49,7 +49,7 @@ module Gitlab
name: queue,
feature_category: get_feature_category,
has_external_dependencies: worker_has_external_dependencies?,
latency_sensitive: latency_sensitive_worker?,
urgency: get_urgency,
resource_boundary: get_worker_resource_boundary,
weight: get_weight,
idempotent: idempotent?
......
......@@ -9,10 +9,10 @@ module Gitlab
private
def create_labels(worker_class, queue)
labels = { queue: queue.to_s, latency_sensitive: FALSE_LABEL, external_dependencies: FALSE_LABEL, feature_category: "", boundary: "" }
labels = { queue: queue.to_s, urgency: "", external_dependencies: FALSE_LABEL, feature_category: "", boundary: "" }
return labels unless worker_class && worker_class.include?(WorkerAttributes)
labels[:latency_sensitive] = bool_as_label(worker_class.latency_sensitive_worker?)
labels[:urgency] = worker_class.get_urgency.to_s
labels[:external_dependencies] = bool_as_label(worker_class.worker_has_external_dependencies?)
feature_category = worker_class.get_feature_category
......
......@@ -124,28 +124,28 @@ describe Gitlab::SidekiqConfig::CliMethods do
name: 'a',
feature_category: :category_a,
has_external_dependencies: false,
latency_sensitive: false,
urgency: :default,
resource_boundary: :cpu
},
{
name: 'a:2',
feature_category: :category_a,
has_external_dependencies: false,
latency_sensitive: true,
urgency: :high,
resource_boundary: :none
},
{
name: 'b',
feature_category: :category_b,
has_external_dependencies: true,
latency_sensitive: true,
urgency: :high,
resource_boundary: :memory
},
{
name: 'c',
feature_category: :category_c,
has_external_dependencies: false,
latency_sensitive: false,
urgency: :none,
resource_boundary: :memory
}
]
......@@ -166,12 +166,12 @@ describe Gitlab::SidekiqConfig::CliMethods do
'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' | %w(a c)
# urgency
'urgency=high' | %w(a:2 b)
'urgency=default' | %w(a)
'urgency=high,default,none' | %w(a a:2 b c)
'urgency=default|urgency=none' | %w(a c)
'urgency!=high' | %w(a c)
# name
'name=a' | %w(a)
......@@ -186,8 +186,8 @@ describe Gitlab::SidekiqConfig::CliMethods do
'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&urgency=high' | %w(a:2)
'feature_category=category_a&urgency=high|feature_category=category_c' | %w(a:2 c)
end
with_them do
......
......@@ -11,7 +11,7 @@ describe Gitlab::SidekiqConfig::Worker do
get_feature_category: attributes[:feature_category],
get_weight: attributes[:weight],
get_worker_resource_boundary: attributes[:resource_boundary],
latency_sensitive_worker?: attributes[:latency_sensitive],
get_urgency: attributes[:urgency],
worker_has_external_dependencies?: attributes[:has_external_dependencies],
idempotent?: attributes[:idempotent]
)
......@@ -47,7 +47,7 @@ describe Gitlab::SidekiqConfig::Worker do
describe 'delegations' do
[
:feature_category_not_owned?, :get_feature_category, :get_weight,
:get_worker_resource_boundary, :latency_sensitive_worker?, :queue,
:get_worker_resource_boundary, :get_urgency, :queue,
:queue_namespace, :worker_has_external_dependencies?
].each do |meth|
it "delegates #{meth} to the worker class" do
......@@ -88,7 +88,7 @@ describe Gitlab::SidekiqConfig::Worker do
attributes_a = {
feature_category: :source_code_management,
has_external_dependencies: false,
latency_sensitive: false,
urgency: :default,
resource_boundary: :memory,
weight: 2,
idempotent: true
......@@ -97,7 +97,7 @@ describe Gitlab::SidekiqConfig::Worker do
attributes_b = {
feature_category: :not_owned,
has_external_dependencies: true,
latency_sensitive: true,
urgency: :high,
resource_boundary: :unknown,
weight: 3,
idempotent: false
......
......@@ -9,7 +9,7 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
let(:queue) { :test }
let(:worker_class) { worker.class }
let(:job) { {} }
let(:default_labels) { { queue: queue.to_s, boundary: "", external_dependencies: "no", feature_category: "", latency_sensitive: "no" } }
let(:default_labels) { { queue: queue.to_s, boundary: "", external_dependencies: "no", feature_category: "", urgency: "default" } }
shared_examples "a metrics client middleware" do
context "with mocked prometheus" do
......@@ -46,17 +46,17 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
it_behaves_like "a metrics client middleware" do
let(:worker) { TestNonAttributedWorker.new }
let(:labels) { default_labels }
let(:labels) { default_labels.merge(urgency: "") }
end
end
context "when workers are attributed" do
def create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, category)
def create_attributed_worker_class(urgency, external_dependencies, resource_boundary, category)
klass = Class.new do
include Sidekiq::Worker
include WorkerAttributes
latency_sensitive_worker! if latency_sensitive
urgency urgency if urgency
worker_has_external_dependencies! if external_dependencies
worker_resource_boundary resource_boundary unless resource_boundary == :unknown
feature_category category unless category.nil?
......@@ -64,17 +64,24 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
stub_const("TestAttributedWorker", klass)
end
let(:latency_sensitive) { false }
let(:urgency) { nil }
let(:external_dependencies) { false }
let(:resource_boundary) { :unknown }
let(:feature_category) { nil }
let(:worker_class) { create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, feature_category) }
let(:worker_class) { create_attributed_worker_class(urgency, external_dependencies, resource_boundary, feature_category) }
let(:worker) { worker_class.new }
context "latency sensitive" do
context "high urgency" do
it_behaves_like "a metrics client middleware" do
let(:latency_sensitive) { true }
let(:labels) { default_labels.merge(latency_sensitive: "yes") }
let(:urgency) { :high }
let(:labels) { default_labels.merge(urgency: "high") }
end
end
context "no urgency" do
it_behaves_like "a metrics client middleware" do
let(:urgency) { :none }
let(:labels) { default_labels.merge(urgency: "none") }
end
end
......@@ -108,11 +115,11 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
context "combined" do
it_behaves_like "a metrics client middleware" do
let(:latency_sensitive) { true }
let(:urgency) { :high }
let(:external_dependencies) { true }
let(:resource_boundary) { :cpu }
let(:feature_category) { :authentication }
let(:labels) { default_labels.merge(latency_sensitive: "yes", external_dependencies: "yes", boundary: "cpu", feature_category: "authentication") }
let(:labels) { default_labels.merge(urgency: "high", external_dependencies: "yes", boundary: "cpu", feature_category: "authentication") }
end
end
end
......
......@@ -11,7 +11,7 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
let(:job) { {} }
let(:job_status) { :done }
let(:labels_with_job_status) { labels.merge(job_status: job_status.to_s) }
let(:default_labels) { { queue: queue.to_s, boundary: "", external_dependencies: "no", feature_category: "", latency_sensitive: "no" } }
let(:default_labels) { { queue: queue.to_s, boundary: "", external_dependencies: "no", feature_category: "", urgency: "default" } }
shared_examples "a metrics middleware" do
context "with mocked prometheus" do
......@@ -130,34 +130,34 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
include Sidekiq::Worker
end
let(:worker) { TestNonAttributedWorker.new }
let(:labels) { default_labels }
let(:labels) { default_labels.merge(urgency: "") }
it_behaves_like "a metrics middleware"
end
context "when workers are attributed" do
def create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, category)
def create_attributed_worker_class(urgency, external_dependencies, resource_boundary, category)
Class.new do
include Sidekiq::Worker
include WorkerAttributes
latency_sensitive_worker! if latency_sensitive
urgency urgency if urgency
worker_has_external_dependencies! if external_dependencies
worker_resource_boundary resource_boundary unless resource_boundary == :unknown
feature_category category unless category.nil?
end
end
let(:latency_sensitive) { false }
let(:urgency) { nil }
let(:external_dependencies) { false }
let(:resource_boundary) { :unknown }
let(:feature_category) { nil }
let(:worker_class) { create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, feature_category) }
let(:worker_class) { create_attributed_worker_class(urgency, external_dependencies, resource_boundary, feature_category) }
let(:worker) { worker_class.new }
context "latency sensitive" do
let(:latency_sensitive) { true }
let(:labels) { default_labels.merge(latency_sensitive: "yes") }
context "high urgency" do
let(:urgency) { :high }
let(:labels) { default_labels.merge(urgency: "high") }
it_behaves_like "a metrics middleware"
end
......@@ -191,11 +191,11 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
end
context "combined" do
let(:latency_sensitive) { true }
let(:urgency) { :none }
let(:external_dependencies) { true }
let(:resource_boundary) { :cpu }
let(:feature_category) { :authentication }
let(:labels) { default_labels.merge(latency_sensitive: "yes", external_dependencies: "yes", boundary: "cpu", feature_category: "authentication") }
let(:labels) { default_labels.merge(urgency: "none", external_dependencies: "yes", boundary: "cpu", feature_category: "authentication") }
it_behaves_like "a metrics middleware"
end
......
......@@ -71,30 +71,30 @@ describe 'Every Sidekiq worker' do
# concurrency, so that each job can consume a large amounts of memory. For this reason, on
# GitLab.com, when a large number of memory-bound jobs arrive at once, we let them queue up
# rather than scaling the hardware to meet the SLO. For this reason, memory-bound,
# latency-sensitive jobs are explicitly discouraged and disabled.
it 'is (exclusively) memory-bound or latency-sentitive, not both', :aggregate_failures do
latency_sensitive_workers = workers_without_defaults
.select(&:latency_sensitive_worker?)
# high urgency jobs are explicitly discouraged and disabled.
it 'is (exclusively) memory-bound or high urgency, not both', :aggregate_failures do
high_urgency_workers = workers_without_defaults
.select { |worker| worker.get_urgency == :high }
latency_sensitive_workers.each do |worker|
expect(worker.get_worker_resource_boundary).not_to eq(:memory), "#{worker.inspect} cannot be both memory-bound and latency sensitive"
high_urgency_workers.each do |worker|
expect(worker.get_worker_resource_boundary).not_to eq(:memory), "#{worker.inspect} cannot be both memory-bound and high urgency"
end
end
# In high traffic installations, such as GitLab.com, `latency_sensitive` workers run in a
# dedicated fleet. In order to ensure short queue times, `latency_sensitive` jobs have strict
# In high traffic installations, such as GitLab.com, `urgency :high` workers run in a
# dedicated fleet. In order to ensure short queue times, `urgency :high` jobs have strict
# SLOs in order to ensure throughput. However, when a worker depends on an external service,
# such as a user's k8s cluster or a third-party internet service, we cannot guarantee latency,
# and therefore throughput. An outage to an 3rd party service could therefore impact throughput
# on other latency_sensitive jobs, leading to degradation through the GitLab application.
# Please see doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies for more
# on other high urgency jobs, leading to degradation through the GitLab application.
# Please see doc/development/sidekiq_style_guide.md#jobs-with-external-dependencies for more
# details.
it 'has (exclusively) external dependencies or is latency-sentitive, not both', :aggregate_failures do
latency_sensitive_workers = workers_without_defaults
.select(&:latency_sensitive_worker?)
it 'has (exclusively) external dependencies or is high urgency, not both', :aggregate_failures do
high_urgency_workers = workers_without_defaults
.select { |worker| worker.get_urgency == :high }
latency_sensitive_workers.each do |worker|
expect(worker.worker_has_external_dependencies?).to be_falsey, "#{worker.inspect} cannot have both external dependencies and be latency sensitive"
high_urgency_workers.each do |worker|
expect(worker.worker_has_external_dependencies?).to be_falsey, "#{worker.inspect} cannot have both external dependencies and be high urgency"
end
end
end
......
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