diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-11-11 18:32:40 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-11-11 18:32:40 +0000 |
commit | 0e1e42885a792145dafc6aa28165d069b1cb5c03 (patch) | |
tree | 2748dddbf1f499ff9be1eca4ce6a8132f44649a1 | |
parent | 52f29501be343ed8cb8bfc8ee64014cd3f4c9ba4 (diff) | |
parent | e840749b84ceb226e46ebdfb489c735e3370cff7 (diff) | |
download | gitlab-ce-0e1e42885a792145dafc6aa28165d069b1cb5c03.tar.gz |
Merge branch 'sidekiq-job-throttling' into 'master'
Allow certain Sidekiq jobs to be throttled
## What does this MR do?
Allows certain slow running Sidekiq jobs to be throttled. It is disabled by default and can be enabled via the Application Settings.
![Screen_Shot_2016-11-04_at_4.51.24_PM](/uploads/a1f1d24c693fcdb278602765cd404d94/Screen_Shot_2016-11-04_at_4.51.24_PM.png)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG.md) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
## What are the relevant issue numbers?
Related to #23352
See merge request !7292
-rw-r--r-- | Gemfile | 1 | ||||
-rw-r--r-- | Gemfile.lock | 3 | ||||
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 5 | ||||
-rw-r--r-- | app/helpers/application_settings_helper.rb | 4 | ||||
-rw-r--r-- | app/models/application_setting.rb | 11 | ||||
-rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 25 | ||||
-rw-r--r-- | changelogs/unreleased/sidekiq-job-throttling.yml | 4 | ||||
-rw-r--r-- | config/initializers/sidekiq.rb | 2 | ||||
-rw-r--r-- | db/migrate/20161103191444_add_sidekiq_throttling_to_application_settings.rb | 31 | ||||
-rw-r--r-- | db/schema.rb | 3 | ||||
-rw-r--r-- | doc/administration/operations.md | 1 | ||||
-rw-r--r-- | doc/administration/operations/img/sidekiq_job_throttling.png | bin | 0 -> 114784 bytes | |||
-rw-r--r-- | doc/administration/operations/sidekiq_job_throttling.md | 33 | ||||
-rw-r--r-- | lib/gitlab/current_settings.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/sidekiq_throttler.rb | 23 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_throttler_spec.rb | 28 |
16 files changed, 178 insertions, 1 deletions
@@ -137,6 +137,7 @@ gem 'acts-as-taggable-on', '~> 4.0' gem 'sidekiq', '~> 4.2' gem 'sidekiq-cron', '~> 0.4.0' gem 'redis-namespace', '~> 1.5.2' +gem 'sidekiq-limit_fetch', '~> 3.4' # HTTP requests gem 'httparty', '~> 0.13.3' diff --git a/Gemfile.lock b/Gemfile.lock index 290e4c3e1b3..81b43f2238a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -685,6 +685,8 @@ GEM redis-namespace (>= 1.5.2) rufus-scheduler (>= 2.0.24) sidekiq (>= 4.0.0) + sidekiq-limit_fetch (3.4.0) + sidekiq (>= 4) simplecov (0.12.0) docile (~> 1.1.0) json (>= 1.8, < 3) @@ -961,6 +963,7 @@ DEPENDENCIES shoulda-matchers (~> 2.8.0) sidekiq (~> 4.2) sidekiq-cron (~> 0.4.0) + sidekiq-limit_fetch (~> 3.4) simplecov (= 0.12.0) slack-notifier (~> 1.2.0) spinach-rails (~> 0.2.1) diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 52e0256943a..b81842e319b 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -117,6 +117,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :send_user_confirmation_email, :container_registry_token_expire_delay, :enabled_git_access_protocol, + :sidekiq_throttling_enabled, + :sidekiq_throttling_factor, :housekeeping_enabled, :housekeeping_bitmaps_enabled, :housekeeping_incremental_repack_period, @@ -125,7 +127,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController repository_storages: [], restricted_visibility_levels: [], import_sources: [], - disabled_oauth_sign_in_sources: [] + disabled_oauth_sign_in_sources: [], + sidekiq_throttling_queues: [] ) end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 45a567a1eba..be5e0301a43 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -100,4 +100,8 @@ module ApplicationSettingsHelper options_for_select(options, @application_setting.repository_storages) end + + def sidekiq_queue_options_for_select + options_for_select(Sidekiq::Queue.all.map(&:name), @application_setting.sidekiq_throttling_queues) + end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index bb60cc8736c..d1e1b45ab43 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -19,6 +19,7 @@ class ApplicationSetting < ActiveRecord::Base serialize :domain_whitelist, Array serialize :domain_blacklist, Array serialize :repository_storages + serialize :sidekiq_throttling_queues, Array cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -85,6 +86,15 @@ class ApplicationSetting < ActiveRecord::Base presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' }, if: :domain_blacklist_enabled? + validates :sidekiq_throttling_factor, + numericality: { greater_than: 0, less_than: 1 }, + presence: { message: 'Throttling factor cannot be empty if Sidekiq Throttling is enabled.' }, + if: :sidekiq_throttling_enabled? + + validates :sidekiq_throttling_queues, + presence: { message: 'Queues to throttle cannot be empty if Sidekiq Throttling is enabled.' }, + if: :sidekiq_throttling_enabled? + validates :housekeeping_incremental_repack_period, presence: true, numericality: { only_integer: true, greater_than: 0 } @@ -180,6 +190,7 @@ class ApplicationSetting < ActiveRecord::Base container_registry_token_expire_delay: 5, repository_storages: ['default'], user_default_external: false, + sidekiq_throttling_enabled: false, housekeeping_enabled: true, housekeeping_bitmaps_enabled: true, housekeeping_incremental_repack_period: 10, diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 450ec322f2c..a236335131a 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -284,6 +284,31 @@ results in fewer but larger UDP packets being sent. %fieldset + %legend Background Jobs + %p + These settings require a restart to take effect. + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :sidekiq_throttling_enabled do + = f.check_box :sidekiq_throttling_enabled + Enable Sidekiq Job Throttling + .help-block + Limit the amount of resources slow running jobs are assigned. + .form-group + = f.label :sidekiq_throttling_queues, 'Sidekiq queues to throttle', class: 'control-label col-sm-2' + .col-sm-10 + = f.select :sidekiq_throttling_queues, sidekiq_queue_options_for_select, { include_hidden: false }, multiple: true, class: 'select2 select-wide', data: { field: 'sidekiq_throttling_queues' } + .help-block + Choose which queues you wish to throttle. + .form-group + = f.label :sidekiq_throttling_factor, 'Throttling Factor', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :sidekiq_throttling_factor, class: 'form-control', min: '0.01', max: '0.99', step: '0.01' + .help-block + The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive. + + %fieldset %legend Spam and Anti-bot Protection .form-group .col-sm-offset-2.col-sm-10 diff --git a/changelogs/unreleased/sidekiq-job-throttling.yml b/changelogs/unreleased/sidekiq-job-throttling.yml new file mode 100644 index 00000000000..ec4e2051c7e --- /dev/null +++ b/changelogs/unreleased/sidekiq-job-throttling.yml @@ -0,0 +1,4 @@ +--- +title: Added ability to throttle Sidekiq Jobs +merge_request: 7292 +author: diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 023af2af23c..b87b31d9697 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -29,6 +29,8 @@ Sidekiq.configure_server do |config| end Sidekiq::Cron::Job.load_from_hash! cron_jobs + Gitlab::SidekiqThrottler.execute! + # Database pool should be at least `sidekiq_concurrency` + 2 # For more info, see: https://github.com/mperham/sidekiq/blob/master/4.0-Upgrade.md config = ActiveRecord::Base.configurations[Rails.env] || diff --git a/db/migrate/20161103191444_add_sidekiq_throttling_to_application_settings.rb b/db/migrate/20161103191444_add_sidekiq_throttling_to_application_settings.rb new file mode 100644 index 00000000000..e644a174964 --- /dev/null +++ b/db/migrate/20161103191444_add_sidekiq_throttling_to_application_settings.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddSidekiqThrottlingToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :application_settings, :sidekiq_throttling_enabled, :boolean, default: false + add_column :application_settings, :sidekiq_throttling_queues, :string + add_column :application_settings, :sidekiq_throttling_factor, :decimal + end +end diff --git a/db/schema.rb b/db/schema.rb index 64d744d8268..3efb884c113 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -98,6 +98,9 @@ ActiveRecord::Schema.define(version: 20161109150329) do t.text "help_page_text_html" t.text "shared_runners_text_html" t.text "after_sign_up_text_html" + t.boolean "sidekiq_throttling_enabled", default: false + t.string "sidekiq_throttling_queues" + t.decimal "sidekiq_throttling_factor" t.boolean "housekeeping_enabled", default: true, null: false t.boolean "housekeeping_bitmaps_enabled", default: true, null: false t.integer "housekeeping_incremental_repack_period", default: 10, null: false diff --git a/doc/administration/operations.md b/doc/administration/operations.md index 4b582d16b64..0daceb98d99 100644 --- a/doc/administration/operations.md +++ b/doc/administration/operations.md @@ -1,6 +1,7 @@ # GitLab operations - [Sidekiq MemoryKiller](operations/sidekiq_memory_killer.md) +- [Sidekiq Job throttling](operations/sidekiq_job_throttling.md) - [Cleaning up Redis sessions](operations/cleaning_up_redis_sessions.md) - [Understanding Unicorn and unicorn-worker-killer](operations/unicorn.md) - [Moving repositories to a new location](operations/moving_repositories.md) diff --git a/doc/administration/operations/img/sidekiq_job_throttling.png b/doc/administration/operations/img/sidekiq_job_throttling.png Binary files differnew file mode 100644 index 00000000000..7f29a4d3c46 --- /dev/null +++ b/doc/administration/operations/img/sidekiq_job_throttling.png diff --git a/doc/administration/operations/sidekiq_job_throttling.md b/doc/administration/operations/sidekiq_job_throttling.md new file mode 100644 index 00000000000..ddeaa22e288 --- /dev/null +++ b/doc/administration/operations/sidekiq_job_throttling.md @@ -0,0 +1,33 @@ +# Sidekiq Job throttling + +> Note: Introduced with GitLab 8.14 + +When your GitLab installation needs to handle tens of thousands of background +jobs, it can be convenient to throttle queues that do not need to be executed +immediately, e.g. long running jobs like Pipelines, thus allowing jobs that do +need to be executed immediately to have access to more resources. + +In order to accomplish this, you can limit the amount of workers that certain +slow running queues can have available. This is what we call Sidekiq Job +Throttling. Depending on your infrastructure, you might have different slow +running queues, which is why you can choose which queues you want to throttle +and by how much you want to throttle them. + +These settings are available in the Application Settings of your GitLab +installation. + +![Sidekiq Job Throttling](img/sidekiq_job_throttling.png) + +The throttle factor determines the maximum number of workers a queue can run on. +This value gets multiplied by `:concurrency` value set in the Sidekiq settings +and rounded up to the closest full integer. + +So, for example, you set the `:concurrency` to 25 and the `Throttling factor` to +0.1, the maximum workers assigned to the selected queues would be 3. + +```ruby +queue_limit = (factor * Sidekiq.options[:concurrency]).ceil +``` + +After enabling the job throttling, you will need to restart your GitLab +instance, in order for the changes to take effect.
\ No newline at end of file diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index ef9160d6437..3a651ef318a 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -23,6 +23,10 @@ module Gitlab settings || fake_application_settings end + def sidekiq_throttling_enabled? + current_application_settings.sidekiq_throttling_enabled + end + def fake_application_settings OpenStruct.new( default_projects_limit: Settings.gitlab['default_projects_limit'], @@ -50,6 +54,7 @@ module Gitlab repository_checks_enabled: true, container_registry_token_expire_delay: 5, user_default_external: false, + sidekiq_throttling_enabled: false, ) end diff --git a/lib/gitlab/sidekiq_throttler.rb b/lib/gitlab/sidekiq_throttler.rb new file mode 100644 index 00000000000..d4d39a888e7 --- /dev/null +++ b/lib/gitlab/sidekiq_throttler.rb @@ -0,0 +1,23 @@ +module Gitlab + class SidekiqThrottler + class << self + def execute! + if Gitlab::CurrentSettings.sidekiq_throttling_enabled? + Gitlab::CurrentSettings.current_application_settings.sidekiq_throttling_queues.each do |queue| + Sidekiq::Queue[queue].limit = queue_limit + end + end + end + + private + + def queue_limit + @queue_limit ||= + begin + factor = Gitlab::CurrentSettings.current_application_settings.sidekiq_throttling_factor + (factor * Sidekiq.options[:concurrency]).ceil + end + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_throttler_spec.rb b/spec/lib/gitlab/sidekiq_throttler_spec.rb new file mode 100644 index 00000000000..ff32e0e699d --- /dev/null +++ b/spec/lib/gitlab/sidekiq_throttler_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::SidekiqThrottler do + before do + Sidekiq.options[:concurrency] = 35 + + stub_application_setting( + sidekiq_throttling_enabled: true, + sidekiq_throttling_factor: 0.1, + sidekiq_throttling_queues: %w[build project_cache] + ) + end + + describe '#execute!' do + it 'sets limits on the selected queues' do + Gitlab::SidekiqThrottler.execute! + + expect(Sidekiq::Queue['build'].limit).to eq 4 + expect(Sidekiq::Queue['project_cache'].limit).to eq 4 + end + + it 'does not set limits on other queues' do + Gitlab::SidekiqThrottler.execute! + + expect(Sidekiq::Queue['merge'].limit).to be_nil + end + end +end |