Use RAILS_MAX_THREADS for client pool size

This is a follow up to sidekiq#2985 (52828e4) adding similar support for the client
connection pool. For Rails servers, Sidekiq is not loaded from the CLI so the
prior change to support setting the concurrency via `RAILS_MAX_THREADS` does
not apply. This means for Rails servers which do not configure a custom size
through an initializer they will run with only the default 5 connections
available to the pool.

When the Rails server runs the initial Redis connection may be made through
`Sidekiq::Client` (e.g. from [`ActiveJob::QueueAdapters::SidekiqAdapter`](https://github.com/rails/rails/blob/v5.1.5/activejob/lib/active_job/queue_adapters/sidekiq_adapter.rb#L20)).
This causes the `redis_pool` to be initialized without any options, setting the
pool size to the default of 5.

    .gem/ruby/2.5.0/gems/sidekiq-5.1.1/lib/sidekiq.rb:125:in `redis_pool'
    .gem/ruby/2.5.0/gems/sidekiq-5.1.1/lib/sidekiq/client.rb:42:in `initialize'
    .gem/ruby/2.5.0/gems/sidekiq-5.1.1/lib/sidekiq/client.rb:131:in `new'
    .gem/ruby/2.5.0/gems/sidekiq-5.1.1/lib/sidekiq/client.rb:131:in `push'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/queue_adapters/sidekiq_adapter.rb:20:in `enqueue'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/enqueuing.rb:51:in `block in enqueue'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:108:in `block in run_callbacks'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/logging.rb:15:in `block (3 levels) in <module:Logging>'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/logging.rb:44:in `block in tag_logger'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:69:in `block in tagged'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:26:in `tagged'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:69:in `tagged'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/logging.rb:44:in `tag_logger'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/logging.rb:14:in `block (2 levels) in <module:Logging>'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:117:in `instance_exec'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:117:in `block in run_callbacks'
    .gem/ruby/2.5.0/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:135:in `run_callbacks'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/enqueuing.rb:47:in `enqueue'
    .gem/ruby/2.5.0/gems/activejob-5.1.5/lib/active_job/enqueuing.rb:18:in `perform_later'

For the majority of cases, a client pool size of 5 is sufficient. However,
servers which utilize a high number of threads, with large job payloads, and
which may experience some network latency issues can see `Timeout::Error`
crashes. This may be further exacerbated by the ~2-20x performance decrease
through `ActiveJob` (sidekiq#3782). Rails addresses this general type of connection
issue for the main database by suggesting that the DB pool size match the
number of threads running. This change applies that logic to the default client
pool size by leveraging the same environment setting; this way there's a
connection available per thread.

This may also have the side effect of a slight performance boost, as there is
less of a chance that threads will be blocked waiting on connections. The
trade-off is that there may be a memory profile increase to handle the
additional Redis connections in the pool; note the pool only creates new
connections as necessary to handle the requests.

Resolves sidekiq#3806

lib/sidekiq/redis_connection.rb

@@ -15,7 +15,15 @@ def create(options={})
         options[:id] = "Sidekiq-#{Sidekiq.server? ? "server" : "client"}-PID-#{$$}" if !options.has_key?(:id)
         options[:url] ||= determine_redis_provider
 
-        size = options[:size] || (Sidekiq.server? ? (Sidekiq.options[:concurrency] + 5) : 5)
+        size = if options[:size]
+                 options[:size]
+               elsif Sidekiq.server?
+                 Sidekiq.options[:concurrency] + 5
+               elsif ENV['RAILS_MAX_THREADS']
+                 Integer(ENV['RAILS_MAX_THREADS'])
+               else
+                 5
+               end
 
         verify_sizing(size, Sidekiq.options[:concurrency]) if Sidekiq.server?
 

test/test_redis_connection.rb

@@ -20,6 +20,61 @@ def client_for(redis)
       assert_equal "Sidekiq-server-PID-#{$$}", pool.checkout.connection.fetch(:id)
     end
 
+    # Readers for these ivars should be available in the next release of
+    # `connection_pool`, until then we need to reach into the internal state to
+    # verify the setting.
+    describe "size" do
+      def client_connection(*args)
+        Sidekiq.stub(:server?, nil) do
+          Sidekiq::RedisConnection.create(*args)
+        end
+      end
+
+      def server_connection(*args)
+        Sidekiq.stub(:server?, "constant") do
+          Sidekiq::RedisConnection.create(*args)
+        end
+      end
+
+      it "uses the specified custom pool size" do
+        pool = client_connection(size: 42)
+        assert_equal 42, pool.instance_eval{ @size }
+        assert_equal 42, pool.instance_eval{ @available.length }
+
+        pool = server_connection(size: 42)
+        assert_equal 42, pool.instance_eval{ @size }
+        assert_equal 42, pool.instance_eval{ @available.length }
+      end
+
+      it "defaults server pool sizes based on concurrency with padding" do
+        _expected_padding = 5
+        Sidekiq.options[:concurrency] = 6
+        pool = server_connection
+
+        assert_equal 11, pool.instance_eval{ @size }
+        assert_equal 11, pool.instance_eval{ @available.length }
+      end
+
+      it "defaults client pool sizes to 5" do
+        pool = client_connection
+
+        assert_equal 5, pool.instance_eval{ @size }
+        assert_equal 5, pool.instance_eval{ @available.length }
+      end
+
+      it "changes client pool sizes with ENV" do
+        begin
+          ENV['RAILS_MAX_THREADS'] = '9'
+          pool = client_connection
+
+          assert_equal 9, pool.instance_eval{ @size }
+          assert_equal 9, pool.instance_eval{ @available.length }
+        ensure
+          ENV.delete('RAILS_MAX_THREADS')
+        end
+      end
+    end
+
     it "disables client setname with nil id" do
       pool = Sidekiq::RedisConnection.create(:id => nil)
       assert_equal Redis, pool.checkout.class