diff options
22 files changed, 25 insertions, 210 deletions
@@ -173,7 +173,6 @@ gem 'acts-as-taggable-on', '~> 5.0' gem 'sidekiq', '~> 5.2.1' gem 'sidekiq-cron', '~> 0.6.0' gem 'redis-namespace', '~> 1.6.0' -gem 'sidekiq-limit_fetch', '~> 3.4', require: false # Cron Parser gem 'rufus-scheduler', '~> 3.4' diff --git a/Gemfile.lock b/Gemfile.lock index 328cc55cb8c..4de78f3ec44 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -846,8 +846,6 @@ GEM sidekiq-cron (0.6.0) rufus-scheduler (>= 3.3.0) sidekiq (>= 4.2.1) - sidekiq-limit_fetch (3.4.0) - sidekiq (>= 4) signet (0.8.1) addressable (~> 2.3) faraday (~> 0.9) @@ -1162,7 +1160,6 @@ DEPENDENCIES shoulda-matchers (~> 3.1.2) sidekiq (~> 5.2.1) sidekiq-cron (~> 0.6.0) - sidekiq-limit_fetch (~> 3.4) simple_po_parser (~> 1.1.2) simplecov (~> 0.14.0) slack-notifier (~> 1.5.1) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index aa6a32fa84e..8f4e1550a52 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -854,8 +854,6 @@ GEM sidekiq-cron (0.6.0) rufus-scheduler (>= 3.3.0) sidekiq (>= 4.2.1) - sidekiq-limit_fetch (3.4.0) - sidekiq (>= 4) signet (0.8.1) addressable (~> 2.3) faraday (~> 0.9) @@ -1172,7 +1170,6 @@ DEPENDENCIES shoulda-matchers (~> 3.1.2) sidekiq (~> 5.2.1) sidekiq-cron (~> 0.6.0) - sidekiq-limit_fetch (~> 3.4) simple_po_parser (~> 1.1.2) simplecov (~> 0.14.0) slack-notifier (~> 1.5.1) diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 875e46969fe..b7c758a42ed 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -128,8 +128,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController disabled_oauth_sign_in_sources: [], import_sources: [], repository_storages: [], - restricted_visibility_levels: [], - sidekiq_throttling_queues: [] + restricted_visibility_levels: [] ] end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index dc393968786..c9a5431d18e 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -108,10 +108,6 @@ module ApplicationSettingsHelper options_for_select(options, selected) end - def sidekiq_queue_options_for_select - options_for_select(Sidekiq::Queue.all.map(&:name), @application_setting.sidekiq_throttling_queues) - end - def circuitbreaker_failure_count_help_text health_link = link_to(s_('AdminHealthPageLink|health page'), admin_health_check_path) api_link = link_to(s_('CircuitBreakerApiLink|circuitbreaker api'), help_page_path("api/repository_storage_health")) @@ -234,9 +230,6 @@ module ApplicationSettingsHelper :session_expire_delay, :shared_runners_enabled, :shared_runners_text, - :sidekiq_throttling_enabled, - :sidekiq_throttling_factor, - :sidekiq_throttling_queues, :sign_in_text, :signup_enabled, :terminal_max_session_time, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 645adddb000..5f835a8da75 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -26,7 +26,6 @@ class ApplicationSetting < ActiveRecord::Base serialize :domain_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize - serialize :sidekiq_throttling_queues, Array # rubocop:disable Cop/ActiveRecordSerialize cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -131,15 +130,6 @@ 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 } @@ -282,7 +272,6 @@ class ApplicationSetting < ActiveRecord::Base send_user_confirmation_email: false, shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], shared_runners_text: nil, - sidekiq_throttling_enabled: false, sign_in_text: nil, signup_enabled: Settings.gitlab['signup_enabled'], terminal_max_session_time: 0, @@ -328,10 +317,6 @@ class ApplicationSetting < ActiveRecord::Base ::Gitlab::Database.cached_column_exists?(:application_settings, :help_page_support_url) end - def sidekiq_throttling_column_exists? - ::Gitlab::Database.cached_column_exists?(:application_settings, :sidekiq_throttling_enabled) - end - def disabled_oauth_sign_in_sources=(sources) sources = (sources || []).map(&:to_s) & Devise.omniauth_providers.map(&:to_s) super(sources) @@ -411,12 +396,6 @@ class ApplicationSetting < ActiveRecord::Base ensure_health_check_access_token! end - def sidekiq_throttling_enabled? - return false unless sidekiq_throttling_column_exists? - - sidekiq_throttling_enabled - end - def usage_ping_can_be_configured? Settings.gitlab.usage_ping_enabled end diff --git a/app/views/admin/application_settings/_background_jobs.html.haml b/app/views/admin/application_settings/_background_jobs.html.haml deleted file mode 100644 index 7d1a64b645a..00000000000 --- a/app/views/admin/application_settings/_background_jobs.html.haml +++ /dev/null @@ -1,27 +0,0 @@ -= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-background-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) - - %fieldset - %p - These settings require a - = link_to 'restart', help_page_path('administration/restart_gitlab') - to take effect. - .form-group - .form-check - = f.check_box :sidekiq_throttling_enabled, class: 'form-check-input' - = f.label :sidekiq_throttling_enabled, class: 'form-check-label' do - Enable Sidekiq Job Throttling - .form-text.text-muted - Limit the amount of resources slow running jobs are assigned. - .form-group - = f.label :sidekiq_throttling_queues, 'Sidekiq queues to throttle', class: 'label-bold' - = f.select :sidekiq_throttling_queues, sidekiq_queue_options_for_select, { include_hidden: false }, multiple: true, class: 'select2 select-wide', data: { field: 'sidekiq_throttling_queues' } - .form-text.text-muted - Choose which queues you wish to throttle. - .form-group - = f.label :sidekiq_throttling_factor, 'Throttling Factor', class: 'label-bold' - = f.number_field :sidekiq_throttling_factor, class: 'form-control', min: '0.01', max: '0.99', step: '0.01' - .form-text.text-muted - The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive. - - = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/admin/application_settings/preferences.html.haml b/app/views/admin/application_settings/preferences.html.haml index 75f76eea3b4..00000b86ab7 100644 --- a/app/views/admin/application_settings/preferences.html.haml +++ b/app/views/admin/application_settings/preferences.html.haml @@ -46,17 +46,6 @@ .settings-content = render 'realtime' -%section.settings.as-background.no-animate#js-background-settings{ class: ('expanded' if expanded_by_default?) } - .settings-header - %h4 - = _('Background jobs') - %button.btn.btn-default.js-settings-toggle{ type: 'button' } - = expanded_by_default? ? _('Collapse') : _('Expand') - %p - = _('Configure Sidekiq job throttling.') - .settings-content - = render 'background_jobs' - %section.settings.as-gitaly.no-animate#js-gitaly-settings{ class: ('expanded' if expanded_by_default?) } .settings-header %h4 diff --git a/changelogs/unreleased/51509-remove-sidekiq-limit-fetch.yml b/changelogs/unreleased/51509-remove-sidekiq-limit-fetch.yml new file mode 100644 index 00000000000..fcc5aae2bda --- /dev/null +++ b/changelogs/unreleased/51509-remove-sidekiq-limit-fetch.yml @@ -0,0 +1,5 @@ +--- +title: Remove background job throttling feature +merge_request: 21748 +author: +type: removed diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 476eaabfed8..6c1079faad1 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -55,8 +55,6 @@ Sidekiq.configure_server do |config| end Sidekiq::Cron::Job.load_from_hash! cron_jobs - Gitlab::SidekiqThrottler.execute! - Gitlab::SidekiqVersioning.install! config = Gitlab::Database.config || diff --git a/db/post_migrate/20180914201132_remove_sidekiq_throttling_from_application_settings.rb b/db/post_migrate/20180914201132_remove_sidekiq_throttling_from_application_settings.rb new file mode 100644 index 00000000000..b3ed0d3f1e9 --- /dev/null +++ b/db/post_migrate/20180914201132_remove_sidekiq_throttling_from_application_settings.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveSidekiqThrottlingFromApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + remove_column :application_settings, :sidekiq_throttling_enabled, :boolean, default: false + remove_column :application_settings, :sidekiq_throttling_queues, :string + remove_column :application_settings, :sidekiq_throttling_factor, :decimal + + Rails.cache.delete("ApplicationSetting:#{Gitlab::VERSION}:#{Rails.version}") + end +end diff --git a/db/schema.rb b/db/schema.rb index b299cde4898..16e3c44513b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180907015926) do +ActiveRecord::Schema.define(version: 20180914201132) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -119,9 +119,6 @@ ActiveRecord::Schema.define(version: 20180907015926) do t.integer "housekeeping_incremental_repack_period", default: 10, null: false t.integer "housekeeping_full_repack_period", default: 50, null: false t.integer "housekeeping_gc_period", default: 200, null: false - t.boolean "sidekiq_throttling_enabled", default: false - t.string "sidekiq_throttling_queues" - t.decimal "sidekiq_throttling_factor" t.boolean "html_emails_enabled", default: true t.string "plantuml_url" t.boolean "plantuml_enabled" diff --git a/doc/administration/index.md b/doc/administration/index.md index 8b6a42b0ca5..702d8e554a8 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -60,7 +60,7 @@ Learn how to install, configure, update, and maintain your GitLab instance. - [Raketasks](../raketasks/README.md): Perform various tasks for maintenance, backups, automatic webhooks setup, etc. - [Backup and restore](../raketasks/backup_restore.md): Backup and restore your GitLab instance. -- [Operations](operations/index.md): Keeping GitLab up and running (clean up Redis sessions, moving repositories, Sidekiq Job throttling, Sidekiq MemoryKiller, Unicorn). +- [Operations](operations/index.md): Keeping GitLab up and running (clean up Redis sessions, moving repositories, Sidekiq MemoryKiller, Unicorn). - [Restart GitLab](restart_gitlab.md): Learn how to restart GitLab and its components. #### Updating GitLab diff --git a/doc/administration/operations/img/sidekiq_job_throttling.png b/doc/administration/operations/img/sidekiq_job_throttling.png Binary files differdeleted file mode 100644 index abd09f3b115..00000000000 --- a/doc/administration/operations/img/sidekiq_job_throttling.png +++ /dev/null diff --git a/doc/administration/operations/index.md b/doc/administration/operations/index.md index e9cad99c4b0..dea98cb8197 100644 --- a/doc/administration/operations/index.md +++ b/doc/administration/operations/index.md @@ -9,8 +9,6 @@ GitLab 7.3 we recommend cleaning up stale sessions to compact the Redis database after you upgrade to GitLab 7.3. - [Moving repositories](moving_repositories.md): Moving all repositories managed by GitLab to another file system or another server. -- [Sidekiq job throttling](sidekiq_job_throttling.md): Throttle Sidekiq queues -that to prioritize important jobs. - [Sidekiq MemoryKiller](sidekiq_memory_killer.md): Configure Sidekiq MemoryKiller to restart Sidekiq. - [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer. diff --git a/doc/administration/operations/sidekiq_job_throttling.md b/doc/administration/operations/sidekiq_job_throttling.md deleted file mode 100644 index ddeaa22e288..00000000000 --- a/doc/administration/operations/sidekiq_job_throttling.md +++ /dev/null @@ -1,33 +0,0 @@ -# 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. - - - -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/doc/api/settings.md b/doc/api/settings.md index d64d65b22f2..1c41b3345ad 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -219,9 +219,6 @@ are listed in the descriptions of the relevant settings. | `session_expire_delay` | integer | no | Session duration in minutes. GitLab restart is required to apply changes | | `shared_runners_enabled` | boolean | no | (**If enabled, requires:** `shared_runners_text`) Enable shared runners for new projects. | | `shared_runners_text` | string | required by: `shared_runners_enabled` | Shared runners text. | -| `sidekiq_throttling_enabled` | boolean | no | (**If enabled, requires:** `sidekiq_throttling_factor` and `sidekiq_throttling_queues`) Enable Sidekiq Job Throttling. | -| `sidekiq_throttling_factor` | decimal | required by: `sidekiq_throttling_enabled` | The factor by which the queues should be throttled. A value between `0.0` and `1.0`, exclusive. | -| `sidekiq_throttling_queues` | array of strings | required by: `sidekiq_throttling_enabled` | Choose which queues you wish to throttle. | | `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`. | diff --git a/lib/api/settings.rb b/lib/api/settings.rb index c80d9890706..8d71bd9dff1 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -117,11 +117,6 @@ module API given shared_runners_enabled: ->(val) { val } do requires :shared_runners_text, type: String, desc: 'Shared runners text ' end - optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' - given sidekiq_throttling_enabled: ->(val) { val } do - requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' - requires :sidekiq_throttling_queues, type: Array[String], desc: 'Choose which queues you wish to throttle' - end optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' diff --git a/lib/gitlab/sidekiq_throttler.rb b/lib/gitlab/sidekiq_throttler.rb deleted file mode 100644 index 5512afa45a8..00000000000 --- a/lib/gitlab/sidekiq_throttler.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Gitlab - class SidekiqThrottler - class << self - def execute! - if Gitlab::CurrentSettings.sidekiq_throttling_enabled? - require 'sidekiq-limit_fetch' - - 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/locale/gitlab.pot b/locale/gitlab.pot index d0e8937fdfc..5663adb0eb3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -787,9 +787,6 @@ msgstr "" msgid "Background color" msgstr "" -msgid "Background jobs" -msgstr "" - msgid "Badges" msgstr "" @@ -1810,9 +1807,6 @@ msgstr "" msgid "Configure Gitaly timeouts." msgstr "" -msgid "Configure Sidekiq job throttling." -msgstr "" - msgid "Configure automatic git checks and housekeeping on repositories." msgstr "" diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 3fb818af8f0..0a69a26eb3e 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -368,16 +368,6 @@ describe 'Admin updates settings' do expect(Gitlab::CurrentSettings.pages_domain_verification_enabled?).to be_truthy expect(page).to have_content "Application settings saved successfully" end - - it 'Change Background jobs settings' do - page.within('.as-background') do - fill_in 'Throttling Factor', with: 1 - click_button 'Save changes' - end - - expect(Gitlab::CurrentSettings.sidekiq_throttling_factor).to eq(1) - expect(page).to have_content "Application settings saved successfully" - end end def check_all_events diff --git a/spec/lib/gitlab/sidekiq_throttler_spec.rb b/spec/lib/gitlab/sidekiq_throttler_spec.rb deleted file mode 100644 index 2dbb7bb7c34..00000000000 --- a/spec/lib/gitlab/sidekiq_throttler_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'spec_helper' - -describe Gitlab::SidekiqThrottler do - describe '#execute!' do - context 'when job throttling is enabled' 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 - - it 'requires sidekiq-limit_fetch' do - expect(described_class).to receive(:require).with('sidekiq-limit_fetch').and_call_original - - described_class.execute! - end - - it 'sets limits on the selected queues' do - described_class.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 - described_class.execute! - - expect(Sidekiq::Queue['merge'].limit).to be_nil - end - end - - context 'when job throttling is disabled' do - it 'does not require sidekiq-limit_fetch' do - expect(described_class).not_to receive(:require).with('sidekiq-limit_fetch') - - described_class.execute! - end - end - end -end |
