Commit 07647a61 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'bvl-sidekiq-job-limit-config' into 'master'

Enable sidekiq job limits by default through application configuration

See merge request gitlab-org/gitlab!68982
parents 5384f326 186465da
......@@ -176,6 +176,16 @@ module ApplicationSettingsHelper
"and the value is encrypted at rest.")
end
def sidekiq_job_limiter_mode_help_text
_("How the job limiter handles jobs exceeding the thresholds specified below. "\
"The 'track' mode only logs the jobs. The 'compress' mode compresses the jobs and "\
"raises an exception if the compressed size exceeds the limit.")
end
def sidekiq_job_limiter_modes_for_select
ApplicationSetting.sidekiq_job_limiter_modes.keys.map { |mode| [mode.humanize, mode] }
end
def visible_attributes
[
:abuse_notification_email,
......@@ -387,7 +397,10 @@ module ApplicationSettingsHelper
:container_registry_cleanup_tags_service_max_list_size,
:keep_latest_artifact,
:whats_new_variant,
:user_deactivation_emails_enabled
:user_deactivation_emails_enabled,
:sidekiq_job_limiter_mode,
:sidekiq_job_limiter_compression_threshold_bytes,
:sidekiq_job_limiter_limit_bytes
].tap do |settings|
settings << :deactivate_dormant_users unless Gitlab.com?
end
......
......@@ -571,6 +571,18 @@ class ApplicationSetting < ApplicationRecord
validates :floc_enabled,
inclusion: { in: [true, false], message: _('must be a boolean value') }
enum sidekiq_job_limiter_mode: {
Gitlab::SidekiqMiddleware::SizeLimiter::Validator::TRACK_MODE => 0,
Gitlab::SidekiqMiddleware::SizeLimiter::Validator::COMPRESS_MODE => 1 # The default
}
validates :sidekiq_job_limiter_mode,
inclusion: { in: self.sidekiq_job_limiter_modes }
validates :sidekiq_job_limiter_compression_threshold_bytes,
numericality: { only_integer: true, greater_than_or_equal_to: 0 }
validates :sidekiq_job_limiter_limit_bytes,
numericality: { only_integer: true, greater_than_or_equal_to: 0 }
attr_encrypted :asset_proxy_secret_key,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,
......
= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-sidekiq-job-limits-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
%fieldset
.form-group
= f.label :sidekiq_job_limiter_mode, _('Limiting mode'), class: 'label-bold'
= f.select :sidekiq_job_limiter_mode, sidekiq_job_limiter_modes_for_select, {}, class: 'form-control'
.form-text.text-muted
= sidekiq_job_limiter_mode_help_text
.form-group
= f.label :sidekiq_job_limiter_compression_threshold_bytes, _('Sidekiq job compression threshold (bytes)'), class: 'label-bold'
= f.number_field :sidekiq_job_limiter_compression_threshold_bytes, class: 'form-control gl-form-input'
.form-text.text-muted
= _('Threshold in bytes at which to compress Sidekiq job arguments.')
.form-group
= f.label :sidekiq_job_limiter_limit_bytes, _('Sidekiq job size limit (bytes)'), class: 'label-bold'
= f.number_field :sidekiq_job_limiter_limit_bytes, class: 'form-control gl-form-input'
.form-text.text-muted
= _("Threshold in bytes at which to reject Sidekiq jobs. Set this to 0 to if you don't want to limit Sidekiq jobs.")
= f.submit _('Save changes'), class: "gl-button btn btn-confirm"
......@@ -82,3 +82,17 @@
= _('Configure the default first day of the week and time tracking units.')
.settings-content
= render 'localization'
%section.settings.as-sidekiq-job-limits.no-animate#js-sidekiq-job-limits-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Sidekiq job size limits')
%button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Limit the size of Sidekiq jobs stored in Redis.')
%span
= link_to _('Learn more.'), help_page_path('user/admin_area/settings/sidekiq_job_limits.md'), target: '_blank', rel: 'noopener noreferrer'
.settings-content
= render 'sidekiq_job_limits'
# frozen_string_literal: true
class AddSidekiqLimitsToApplicationSettings < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! # needed for now to avoid subtransactions
def up
with_lock_retries do
add_column :application_settings, :sidekiq_job_limiter_mode, :smallint, default: 1, null: false
add_column :application_settings, :sidekiq_job_limiter_compression_threshold_bytes, :integer, default: 100_000, null: false
add_column :application_settings, :sidekiq_job_limiter_limit_bytes, :integer, default: 0, null: false
end
end
def down
with_lock_retries do
remove_column :application_settings, :sidekiq_job_limiter_mode
remove_column :application_settings, :sidekiq_job_limiter_compression_threshold_bytes
remove_column :application_settings, :sidekiq_job_limiter_limit_bytes
end
end
end
a8dc6d1fecf7b26182dd89f4dae088fb315774ff4720c282f608bd0c45c75a41
\ No newline at end of file
......@@ -10340,6 +10340,9 @@ CREATE TABLE application_settings (
throttle_unauthenticated_api_enabled boolean DEFAULT false NOT NULL,
throttle_unauthenticated_api_requests_per_period integer DEFAULT 3600 NOT NULL,
throttle_unauthenticated_api_period_in_seconds integer DEFAULT 3600 NOT NULL,
sidekiq_job_limiter_mode smallint DEFAULT 1 NOT NULL,
sidekiq_job_limiter_compression_threshold_bytes integer DEFAULT 100000 NOT NULL,
sidekiq_job_limiter_limit_bytes integer DEFAULT 0 NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
......@@ -386,6 +386,9 @@ listed in the descriptions of the relevant settings.
| `shared_runners_enabled` | boolean | no | (**If enabled, requires:** `shared_runners_text` and `shared_runners_minutes`) Enable shared runners for new projects. |
| `shared_runners_minutes` | integer | required by: `shared_runners_enabled` | **(PREMIUM)** Set the maximum number of pipeline minutes that a group can use on shared runners per month. |
| `shared_runners_text` | string | required by: `shared_runners_enabled` | Shared runners text. |
| `sidekiq_job_limiter_mode` | string | no | `track` or `compress`. Sets the behavior for [Sidekiq job size limits](../user/admin_area/settings/sidekiq_job_limits.md). Default: 'compress'. |
| `sidekiq_job_limiter_compression_threshold_bytes` | integer | no | The threshold in bytes at which Sidekiq jobs are compressed before being stored in Redis. Default: 100 000 bytes (100KB). |
| `sidekiq_job_limiter_limit_bytes` | integer | no | The threshold in bytes at which Sidekiq jobs are rejected. Default: 0 bytes (doesn't reject any job). |
| `sign_in_text` | string | no | Text on the login page. |
| `signin_enabled` | string | no | (Deprecated: Use `password_authentication_enabled_for_web` instead) Flag indicating if password authentication is enabled for the web interface. |
| `signup_enabled` | boolean | no | Enable registration. Default is `true`. |
......
......@@ -120,6 +120,7 @@ To access the default page for Admin Area settings:
| [Polling interval multiplier](../../../administration/polling.md) | Configure how frequently the GitLab UI polls for updates. |
| [Gitaly timeouts](gitaly_timeouts.md) | Configure Gitaly timeouts. |
| Localization | [Default first day of the week](../../profile/preferences.md) and [Time tracking](../../project/time_tracking.md#limit-displayed-units-to-hours). |
| [Sidekiq Job Limits](sidekiq_job_limits.md) | Limit the size of Sidekiq jobs stored in Redis. |
### Default first day of the week
......
---
stage: none
group: unassigned
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
type: reference
---
# Sidekiq job size limits **(FREE SELF)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68982) in GitLab 14.3.
[Sidekiq](../../../administration/sidekiq.md) jobs get stored in
Redis. To avoid excessive memory for Redis, we:
- Compress job arguments before storing them in Redis.
arguments before storing them in Redis, and rejecting jobs that exceed
- Reject jobs that exceed the specified threshold limit after compression.
To access Sidekiq job size limits:
1. On the top bar, select **Menu >** **{admin}** **Admin**.
1. On the left sidebar, select **Settings > Preferences**.
1. Expand **Sidekiq job size limits**.
1. Adjust the compression threshold or size limit. The compression can
be disabled by selecting "Track" mode.
## Available settings
| Setting | Default | Description |
|-------------------------------------------|------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Limiting mode | Compress | This mode compresses the jobs at the specified threshold and rejects them if they exceed the specified limit after compression. |
| Sidekiq job compression threshold (bytes) | 100 000 (100 KB) | When the size of arguments exceeds this threshold, they are compressed before being stored in Redis. |
| Sidekiq job size limit (bytes) | 0 | The jobs exceeding this size after compression are rejected. This avoids excessive memory usage in Redis leading to instability. Setting it to 0 prevents rejecting jobs. |
After changing these values, [restart
Sidekiq](../../../administration/restart_gitlab.md).
......@@ -4,12 +4,12 @@ module Gitlab
module SidekiqMiddleware
module SizeLimiter
# Handle a Sidekiq job payload limit based on current configuration.
# This validator pulls the configuration from the environment variables:
# - GITLAB_SIDEKIQ_SIZE_LIMITER_MODE: the current mode of the size
# limiter. This must be either `track` or `compress`.
# - GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES: the
# threshold before the input job payload is compressed.
# - GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES: the size limit in bytes.
# This validator pulls the configuration from application settings:
# - limiter_mode: the current mode of the size
# limiter. This must be either `track` or `compress`.
# - compression_threshold_bytes: the threshold before the input job
# payload is compressed.
# - limit_bytes: the size limit in bytes.
#
# In track mode, if a job payload limit exceeds the size limit, an
# event is sent to Sentry and the job is scheduled like normal.
......@@ -18,12 +18,29 @@ module Gitlab
# then compressed. If the compressed payload still exceeds the limit, the
# job is discarded, and a ExceedLimitError exception is raised.
class Validator
def self.validate!(worker_class, job)
new(worker_class, job).validate!
# Avoid limiting the size of jobs for `BackgroundMigrationWorker` classes.
# We can't read the configuration from `ApplicationSetting` for those jobs
# when migrating a path that modifies the `application_settings` table.
# Reading the application settings through `ApplicationSetting#current`
# causes a `SELECT` with a list of column names, but that list of column
# names might not match what the table currently looks like causing
# an error when scheduling background migrations.
#
# The worker classes aren't constants here, because that would force
# Application Settings to be loaded earlier causing failures loading
# the environmant in rake tasks
EXEMPT_WORKER_NAMES = ["BackgroundMigrationWorker", "Database::BatchedBackgroundMigrationWorker"].to_set
class << self
def validate!(worker_class, job)
return if EXEMPT_WORKER_NAMES.include?(worker_class.to_s)
new(worker_class, job).validate!
end
end
DEFAULT_SIZE_LIMIT = 0
DEFAULT_COMPRESION_THRESHOLD_BYTES = 100_000 # 100kb
DEFAULT_COMPRESSION_THRESHOLD_BYTES = 100_000 # 100kb
MODES = [
TRACK_MODE = 'track',
......@@ -34,9 +51,9 @@ module Gitlab
def initialize(
worker_class, job,
mode: ENV['GITLAB_SIDEKIQ_SIZE_LIMITER_MODE'],
compression_threshold: ENV['GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES'],
size_limit: ENV['GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES']
mode: Gitlab::CurrentSettings.sidekiq_job_limiter_mode,
compression_threshold: Gitlab::CurrentSettings.sidekiq_job_limiter_compression_threshold_bytes,
size_limit: Gitlab::CurrentSettings.sidekiq_job_limiter_limit_bytes
)
@worker_class = worker_class
@job = job
......@@ -72,10 +89,10 @@ module Gitlab
end
def set_compression_threshold(compression_threshold)
@compression_threshold = (compression_threshold || DEFAULT_COMPRESION_THRESHOLD_BYTES).to_i
@compression_threshold = (compression_threshold || DEFAULT_COMPRESSION_THRESHOLD_BYTES).to_i
if @compression_threshold <= 0
::Sidekiq.logger.warn "Invalid Sidekiq size limiter compression threshold: #{@compression_threshold}"
@compression_threshold = DEFAULT_COMPRESION_THRESHOLD_BYTES
@compression_threshold = DEFAULT_COMPRESSION_THRESHOLD_BYTES
end
end
......@@ -83,7 +100,7 @@ module Gitlab
@size_limit = (size_limit || DEFAULT_SIZE_LIMIT).to_i
if @size_limit < 0
::Sidekiq.logger.warn "Invalid Sidekiq size limiter limit: #{@size_limit}"
@size_limit = 0
@size_limit = DEFAULT_SIZE_LIMIT
end
end
......
......@@ -16778,6 +16778,9 @@ msgstr ""
msgid "How many seconds an IP will be counted towards the limit"
msgstr ""
msgid "How the job limiter handles jobs exceeding the thresholds specified below. The 'track' mode only logs the jobs. The 'compress' mode compresses the jobs and raises an exception if the compressed size exceeds the limit."
msgstr ""
msgid "I accept the %{terms_link}"
msgstr ""
......@@ -20205,11 +20208,17 @@ msgstr ""
msgid "Limit the number of issues and epics per minute a user can create through web and API requests."
msgstr ""
msgid "Limit the size of Sidekiq jobs stored in Redis."
msgstr ""
msgid "Limited to showing %d event at most"
msgid_plural "Limited to showing %d events at most"
msgstr[0] ""
msgstr[1] ""
msgid "Limiting mode"
msgstr ""
msgid "Line changes"
msgstr ""
......@@ -31070,6 +31079,15 @@ msgstr ""
msgid "Sidebar|Weight"
msgstr ""
msgid "Sidekiq job compression threshold (bytes)"
msgstr ""
msgid "Sidekiq job size limit (bytes)"
msgstr ""
msgid "Sidekiq job size limits"
msgstr ""
msgid "Sign in"
msgstr ""
......@@ -34793,6 +34811,12 @@ msgstr ""
msgid "ThreatMonitoring|View documentation"
msgstr ""
msgid "Threshold in bytes at which to compress Sidekiq job arguments."
msgstr ""
msgid "Threshold in bytes at which to reject Sidekiq jobs. Set this to 0 to if you don't want to limit Sidekiq jobs."
msgstr ""
msgid "Throughput"
msgstr ""
......
......@@ -284,4 +284,10 @@ RSpec.describe ApplicationSettingsHelper do
end
end
end
describe '#sidekiq_job_limiter_modes_for_select' do
subject { helper.sidekiq_job_limiter_modes_for_select }
it { is_expected.to eq([%w(Track track), %w(Compress compress)]) }
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_failures do
let(:base_payload) do
{
"class" => "ARandomWorker",
......@@ -31,10 +31,35 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
end
before do
# Settings aren't in the database in specs, but stored in memory, this is fine
# for these tests.
allow(Gitlab::CurrentSettings).to receive(:current_application_settings?).and_return(true)
stub_const("TestSizeLimiterWorker", worker_class)
end
describe '#initialize' do
context 'configuration from application settings' do
let(:validator) { described_class.new(worker_class, job_payload) }
it 'has the right defaults' do
expect(validator.mode).to eq(described_class::COMPRESS_MODE)
expect(validator.compression_threshold).to eq(described_class::DEFAULT_COMPRESSION_THRESHOLD_BYTES)
expect(validator.size_limit).to eq(described_class::DEFAULT_SIZE_LIMIT)
end
it 'allows configuration through application settings' do
stub_application_setting(
sidekiq_job_limiter_mode: 'track',
sidekiq_job_limiter_compression_threshold_bytes: 1,
sidekiq_job_limiter_limit_bytes: 2
)
expect(validator.mode).to eq(described_class::TRACK_MODE)
expect(validator.compression_threshold).to eq(1)
expect(validator.size_limit).to eq(2)
end
end
context 'when the input mode is valid' do
it 'does not log a warning message' do
expect(::Sidekiq.logger).not_to receive(:warn)
......@@ -58,7 +83,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
it 'defaults to track mode' do
expect(::Sidekiq.logger).not_to receive(:warn)
validator = described_class.new(TestSizeLimiterWorker, job_payload)
validator = described_class.new(TestSizeLimiterWorker, job_payload, mode: nil)
expect(validator.mode).to eql('track')
end
......@@ -74,7 +99,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
end
context 'when the size input is invalid' do
it 'defaults to 0 and logs a warning message' do
it 'logs a warning message' do
expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter limit: -1')
validator = described_class.new(TestSizeLimiterWorker, job_payload, size_limit: -1)
......@@ -87,9 +112,9 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
it 'defaults to 0' do
expect(::Sidekiq.logger).not_to receive(:warn)
validator = described_class.new(TestSizeLimiterWorker, job_payload)
validator = described_class.new(TestSizeLimiterWorker, job_payload, size_limit: nil)
expect(validator.size_limit).to be(0)
expect(validator.size_limit).to be(described_class::DEFAULT_SIZE_LIMIT)
end
end
......@@ -318,20 +343,30 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
end
end
describe '#validate!' do
context 'when calling SizeLimiter.validate!' do
let(:validate) { ->(worker_clas, job) { described_class.validate!(worker_class, job) } }
describe '.validate!' do
let(:validate) { ->(worker_class, job) { described_class.validate!(worker_class, job) } }
it_behaves_like 'validate limit job payload size' do
before do
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_MODE', mode)
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES', size_limit)
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES', compression_threshold)
stub_application_setting(
sidekiq_job_limiter_mode: mode,
sidekiq_job_limiter_compression_threshold_bytes: compression_threshold,
sidekiq_job_limiter_limit_bytes: size_limit
)
end
end
it_behaves_like 'validate limit job payload size'
it "skips background migrations" do
expect(described_class).not_to receive(:new)
described_class::EXEMPT_WORKER_NAMES.each do |class_name|
validate.call(class_name.constantize, job_payload)
end
end
end
context 'when creating an instance with the related ENV variables' do
describe '#validate!' do
context 'when creating an instance with the related configuration variables' do
let(:validate) do
->(worker_clas, job) do
described_class.new(worker_class, job).validate!
......@@ -339,9 +374,11 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do
end
before do
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_MODE', mode)
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES', size_limit)
stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES', compression_threshold)
stub_application_setting(
sidekiq_job_limiter_mode: mode,
sidekiq_job_limiter_compression_threshold_bytes: compression_threshold,
sidekiq_job_limiter_limit_bytes: size_limit
)
end
it_behaves_like 'validate limit job payload size'
......
......@@ -956,6 +956,20 @@ RSpec.describe ApplicationSetting do
it { is_expected.not_to allow_value(nil).for(throttle_setting) }
end
end
context 'sidekiq job limiter settings' do
it 'has the right defaults', :aggregate_failures do
expect(setting.sidekiq_job_limiter_mode).to eq('compress')
expect(setting.sidekiq_job_limiter_compression_threshold_bytes)
.to eq(Gitlab::SidekiqMiddleware::SizeLimiter::Validator::DEFAULT_COMPRESSION_THRESHOLD_BYTES)
expect(setting.sidekiq_job_limiter_limit_bytes)
.to eq(Gitlab::SidekiqMiddleware::SizeLimiter::Validator::DEFAULT_SIZE_LIMIT)
end
it { is_expected.to allow_value('track').for(:sidekiq_job_limiter_mode) }
it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_compression_threshold_bytes).only_integer.is_greater_than_or_equal_to(0) }
it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_limit_bytes).only_integer.is_greater_than_or_equal_to(0) }
end
end
context 'restrict creating duplicates' do
......
......@@ -594,5 +594,20 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
expect(json_response['error']).to eq('whats_new_variant does not have a valid value')
end
end
context 'sidekiq job limit settings' do
it 'updates the settings' do
settings = {
sidekiq_job_limiter_mode: 'track',
sidekiq_job_limiter_compression_threshold_bytes: 1,
sidekiq_job_limiter_limit_bytes: 2
}.stringify_keys
put api("/application/settings", admin), params: settings
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.slice(*settings.keys)).to eq(settings)
end
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