Refactored Sidekiq Throttler and updated documentationsidekiq-job-throttling

6 files changed, 20 insertions, 23 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 075e4f4fc9d..d1e1b45ab43 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -19,7 +19,7 @@ class ApplicationSetting < ActiveRecord::Base
   serialize :domain_whitelist, Array
   serialize :domain_blacklist, Array
   serialize :repository_storages
-  serialize :sidekiq_throttling_queues
+  serialize :sidekiq_throttling_queues, Array
 
   cache_markdown_field :sign_in_text
   cache_markdown_field :help_page_text
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 9b1b3f0e16e..a236335131a 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -304,9 +304,9 @@
     .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', max: '0.99', step: '0.01'
+        = 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.1 and 0.9.
+          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
diff --git a/changelogs/unreleased/sidekiq-job-throttling.yml b/changelogs/unreleased/sidekiq-job-throttling.yml
index 1f3aad7ae96..ec4e2051c7e 100644
--- a/changelogs/unreleased/sidekiq-job-throttling.yml
+++ b/changelogs/unreleased/sidekiq-job-throttling.yml
@@ -1,4 +1,4 @@
 ---
 title: Added ability to throttle Sidekiq Jobs
 merge_request: 7292
-author: Patricio Cano
+author:
diff --git a/doc/administration/operations/sidekiq_job_throttling.md b/doc/administration/operations/sidekiq_job_throttling.md
index 33cedee7ebd..ddeaa22e288 100644
--- a/doc/administration/operations/sidekiq_job_throttling.md
+++ b/doc/administration/operations/sidekiq_job_throttling.md
@@ -3,14 +3,15 @@
 > Note: Introduced with GitLab 8.14
 
 When your GitLab installation needs to handle tens of thousands of background
-jobs, it can be convenient to prioritize queues that need to be executed
-immediately, e.g. user initiated actions like merging a Merge Request.
+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 get can have available. This is what we call Sidekiq Job
+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 to throttle and by
-how much you want to throttle them.
+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.
@@ -24,8 +25,8 @@ 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.
 
-```
-limit = (factor * Sidekiq.options[:concurrency]).ceil
+```ruby
+queue_limit = (factor * Sidekiq.options[:concurrency]).ceil
 ```
 
 After enabling the job throttling, you will need to restart your GitLab
diff --git a/lib/gitlab/sidekiq_throttler.rb b/lib/gitlab/sidekiq_throttler.rb
index 771736e7606..d4d39a888e7 100644
--- a/lib/gitlab/sidekiq_throttler.rb
+++ b/lib/gitlab/sidekiq_throttler.rb
@@ -3,18 +3,20 @@ module Gitlab
     class << self
       def execute!
         if Gitlab::CurrentSettings.sidekiq_throttling_enabled?
-          current_application_settings.sidekiq_throttling_queues.each do |queue|
-            Sidekiq::Queue[queue].limit = set_limit
+          Gitlab::CurrentSettings.current_application_settings.sidekiq_throttling_queues.each do |queue|
+            Sidekiq::Queue[queue].limit = queue_limit
           end
         end
       end
 
       private
 
-      def set_limit
-        factor = current_application_settings.sidekiq_throttling_factor
-
-        (factor * Sidekiq.options[:concurrency]).ceil
+      def queue_limit
+        @queue_limit ||=
+          begin
+            factor = Gitlab::CurrentSettings.current_application_settings.sidekiq_throttling_factor
+            (factor * Sidekiq.options[:concurrency]).ceil
+          end
       end
     end
   end
diff --git a/spec/lib/gitlab/sidekiq_throttler_spec.rb b/spec/lib/gitlab/sidekiq_throttler_spec.rb
index ac4a64c0f43..ff32e0e699d 100644
--- a/spec/lib/gitlab/sidekiq_throttler_spec.rb
+++ b/spec/lib/gitlab/sidekiq_throttler_spec.rb
@@ -11,12 +11,6 @@ describe Gitlab::SidekiqThrottler do
     )
   end
 
-  describe '#set_limit' do
-    it 'returns the correct limit' do
-      expect(Gitlab::SidekiqThrottler.send(:set_limit)).to eq 4
-    end
-  end
-
   describe '#execute!' do
     it 'sets limits on the selected queues' do
       Gitlab::SidekiqThrottler.execute!