Commit 0db5f576 authored by Sean McGivern's avatar Sean McGivern

Only require sidekiq-limit_fetch when enabled in settings

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.
parent fa293df2
......@@ -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'
......
......@@ -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
......
---
title: Only require Sidekiq throttling library when enabled, to reduce cache misses
merge_request:
author:
type: fixed
......@@ -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
......
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment