From 640a4c88341f790ede78f845d29a37634030581f Mon Sep 17 00:00:00 2001
From: Paco Guzman <pacoguzmanp@gmail.com>
Date: Mon, 3 Oct 2016 16:42:02 +0200
Subject: [PATCH] Use higher size on Gitlab::Redis connection pool on Sidekiq
 servers

---
 CHANGELOG                     |  1 +
 lib/gitlab/redis.rb           | 12 +++++++++++-
 spec/lib/gitlab/redis_spec.rb | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 68962f20d0b..649215db31c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -37,6 +37,7 @@ v 8.13.0 (unreleased)
   - Append issue template to existing description !6149 (Joseph Frazier)
   - Trending projects now only show public projects and the list of projects is cached for a day
   - Revoke button in Applications Settings underlines on hover.
+  - Use higher size on Gitlab::Redis connection pool on Sidekiq servers
   - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska)
   - Fix Long commit messages overflow viewport in file tree
   - Revert avoid touching file system on Build#artifacts?
diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb
index 3faab937726..c649da8c426 100644
--- a/lib/gitlab/redis.rb
+++ b/lib/gitlab/redis.rb
@@ -24,10 +24,20 @@ module Gitlab
       end
 
       def with
-        @pool ||= ConnectionPool.new { ::Redis.new(params) }
+        @pool ||= ConnectionPool.new(size: pool_size) { ::Redis.new(params) }
         @pool.with { |redis| yield redis }
       end
 
+      def pool_size
+        if Sidekiq.server?
+          # the pool will be used in a multi-threaded context
+          Sidekiq.options[:concurrency] + 5
+        else
+          # probably this is a Unicorn process, so single threaded
+          5
+        end
+      end
+
       def _raw_config
         return @_raw_config if defined?(@_raw_config)
 
diff --git a/spec/lib/gitlab/redis_spec.rb b/spec/lib/gitlab/redis_spec.rb
index cb54c020b31..74ff85e132a 100644
--- a/spec/lib/gitlab/redis_spec.rb
+++ b/spec/lib/gitlab/redis_spec.rb
@@ -88,6 +88,34 @@ describe Gitlab::Redis do
     end
   end
 
+  describe '.with' do
+    before { clear_pool }
+    after { clear_pool }
+
+    context 'when running not on sidekiq workers' do
+      before { allow(Sidekiq).to receive(:server?).and_return(false) }
+
+      it 'instantiates a connection pool with size 5' do
+        expect(ConnectionPool).to receive(:new).with(size: 5).and_call_original
+
+        described_class.with { |_redis| true }
+      end
+    end
+
+    context 'when running on sidekiq workers' do
+      before do
+        allow(Sidekiq).to receive(:server?).and_return(true)
+        allow(Sidekiq).to receive(:options).and_return({ concurrency: 18 })
+      end
+
+      it 'instantiates a connection pool with a size based on the concurrency of the worker' do
+        expect(ConnectionPool).to receive(:new).with(size: 18 + 5).and_call_original
+
+        described_class.with { |_redis| true }
+      end
+    end
+  end
+
   describe '#raw_config_hash' do
     it 'returns default redis url when no config file is present' do
       expect(subject).to receive(:fetch_config) { false }
@@ -114,4 +142,10 @@ describe Gitlab::Redis do
   rescue NameError
     # raised if @_raw_config was not set; ignore
   end
+
+  def clear_pool
+    described_class.remove_instance_variable(:@pool)
+  rescue NameError
+    # raised if @pool was not set; ignore
+  end
 end