Only require sidekiq-limit_fetch when enabled in settingsonly-limit-fetch-when-requested

This gem allows Sidekiq jobs to be throttled. Unfortunately, it has a
side-effect: when we haven't enabled job throttling, it will still hit Redis a
lot (and miss, because nothing is configured).

As this setting already required a restart, ensure that the library is only
required when it's enabled.

5 files changed, 44 insertions, 19 deletions

diff --git a/Gemfile b/Gemfile
index 6c8f64bfded..a0a9dddac10 100644
--- a/Gemfile
+++ b/Gemfile
@@ -152,7 +152,7 @@ gem 'acts-as-taggable-on', '~> 4.0'
 gem 'sidekiq', '~> 5.0'
 gem 'sidekiq-cron', '~> 0.6.0'
 gem 'redis-namespace', '~> 1.5.2'
-gem 'sidekiq-limit_fetch', '~> 3.4'
+gem 'sidekiq-limit_fetch', '~> 3.4', require: false
 
 # Cron Parser
 gem 'rufus-scheduler', '~> 3.4'
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 8bf6556079b..dc585054316 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -362,7 +362,9 @@
   %fieldset
     %legend Background Jobs
     %p
-      These settings require a restart to take effect.
+      These settings require a
+      = link_to 'restart', help_page_path('administration/restart_gitlab')
+      to take effect.
     .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox
diff --git a/changelogs/unreleased/only-limit-fetch-when-requested.yml b/changelogs/unreleased/only-limit-fetch-when-requested.yml
new file mode 100644
index 00000000000..d9acdf56511
--- /dev/null
+++ b/changelogs/unreleased/only-limit-fetch-when-requested.yml
@@ -0,0 +1,5 @@
+---
+title: Only require Sidekiq throttling library when enabled, to reduce cache misses
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/sidekiq_throttler.rb b/lib/gitlab/sidekiq_throttler.rb
index d4d39a888e7..5512afa45a8 100644
--- a/lib/gitlab/sidekiq_throttler.rb
+++ b/lib/gitlab/sidekiq_throttler.rb
@@ -3,6 +3,8 @@ module Gitlab
     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
diff --git a/spec/lib/gitlab/sidekiq_throttler_spec.rb b/spec/lib/gitlab/sidekiq_throttler_spec.rb
index 6374ac80207..2dbb7bb7c34 100644
--- a/spec/lib/gitlab/sidekiq_throttler_spec.rb
+++ b/spec/lib/gitlab/sidekiq_throttler_spec.rb
@@ -1,28 +1,44 @@
 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
-      described_class.execute!
+    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['build'].limit).to eq 4
-      expect(Sidekiq::Queue['project_cache'].limit).to eq 4
+        expect(Sidekiq::Queue['merge'].limit).to be_nil
+      end
     end
 
-    it 'does not set limits on other queues' do
-      described_class.execute!
+    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')
 
-      expect(Sidekiq::Queue['merge'].limit).to be_nil
+        described_class.execute!
+      end
     end
   end
 end