Merge branch '51509-remove-sidekiq-limit-fetch' into 'master'

Remove background job throttling feature

Closes #51509

See merge request gitlab-org/gitlab-ce!21748

Gemfile

@@ -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'

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)

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)

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

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,

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

app/views/admin/application_settings/_background_jobs.html.haml

-= 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"

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

changelogs/unreleased/51509-remove-sidekiq-limit-fetch.yml

+---
+title: Remove background job throttling feature
+merge_request: 21748
+author:
+type: removed

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 ||

db/post_migrate/20180914201132_remove_sidekiq_throttling_from_application_settings.rb

+# 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

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"

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

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.

doc/administration/operations/sidekiq_job_throttling.md

-# 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

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`. |

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'

lib/gitlab/sidekiq_throttler.rb

-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

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 ""
 

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

spec/lib/gitlab/sidekiq_throttler_spec.rb

-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