From 5a4de159f0e1f457bdb90d4f2a8552623fa9b6a0 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 16 Jul 2013 15:40:50 -0600 Subject: [PATCH 1/2] Allow setting of a custom timeout for the Redis Connection Pool --- .gitignore | 2 ++ lib/sidekiq/redis_connection.rb | 3 ++- test/test_redis_connection.rb | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 9b6ea19d..4b765ccc 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,6 @@ Gemfile.lock dump.rdb .rbx coverage/ +vendor/ +.bundle/ .sass-cache/ diff --git a/lib/sidekiq/redis_connection.rb b/lib/sidekiq/redis_connection.rb index 6143d0af..c029af3a 100644 --- a/lib/sidekiq/redis_connection.rb +++ b/lib/sidekiq/redis_connection.rb @@ -9,10 +9,11 @@ module Sidekiq url = options[:url] || determine_redis_provider || 'redis://localhost:6379/0' # need a connection for Fetcher and Retry size = options[:size] || (Sidekiq.server? ? (Sidekiq.options[:concurrency] + 2) : 5) + timeout = options[:timeout] || 1 log_info(url, options) - ConnectionPool.new(:timeout => 1, :size => size) do + ConnectionPool.new(:timeout => timeout, :size => size) do build_client(url, options[:namespace], options[:driver] || 'ruby') end end diff --git a/test/test_redis_connection.rb b/test/test_redis_connection.rb index 9a234555..cb6de0a7 100644 --- a/test/test_redis_connection.rb +++ b/test/test_redis_connection.rb @@ -22,6 +22,20 @@ class TestRedisConnection < Minitest::Test assert_equal "yyy", pool.checkout.namespace end end + + describe "timeout" do + it "uses a given :timeout over the default of 1" do + pool = Sidekiq::RedisConnection.create(:timeout => 5) + + assert_equal 5, pool.instance_eval{ @timeout } + end + + it "uses the default timeout of 1 if no override" do + pool = Sidekiq::RedisConnection.create + + assert_equal 1, pool.instance_eval{ @timeout } + end + end end describe ".determine_redis_provider" do From 48c70f85451e94645783fab5d68c0716693c809d Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 17 Jul 2013 09:02:51 -0600 Subject: [PATCH 2/2] Added option to set the network timeout for the Redis connection --- lib/sidekiq/redis_connection.rb | 18 +++++++++++++----- test/test_redis_connection.rb | 20 ++++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/sidekiq/redis_connection.rb b/lib/sidekiq/redis_connection.rb index c029af3a..e06fa221 100644 --- a/lib/sidekiq/redis_connection.rb +++ b/lib/sidekiq/redis_connection.rb @@ -9,19 +9,19 @@ module Sidekiq url = options[:url] || determine_redis_provider || 'redis://localhost:6379/0' # need a connection for Fetcher and Retry size = options[:size] || (Sidekiq.server? ? (Sidekiq.options[:concurrency] + 2) : 5) - timeout = options[:timeout] || 1 + pool_timeout = options[:pool_timeout] || 1 log_info(url, options) - ConnectionPool.new(:timeout => timeout, :size => size) do - build_client(url, options[:namespace], options[:driver] || 'ruby') + ConnectionPool.new(:timeout => pool_timeout, :size => size) do + build_client(url, options[:namespace], options[:driver] || 'ruby', options[:network_timeout]) end end private - def build_client(url, namespace, driver) - client = Redis.connect(:url => url, :driver => driver) + def build_client(url, namespace, driver, network_timeout) + client = Redis.connect client_opts(url, driver, network_timeout) if namespace require 'redis/namespace' Redis::Namespace.new(namespace, :redis => client) @@ -30,6 +30,14 @@ module Sidekiq end end + def client_opts(url, driver, timeout) + if timeout + { :url => url, :driver => driver, :timeout => timeout } + else + { :url => url, :driver => driver } + end + end + def log_info(url, options) opts = options.dup opts.delete(:url) diff --git a/test/test_redis_connection.rb b/test/test_redis_connection.rb index cb6de0a7..9bf4099f 100644 --- a/test/test_redis_connection.rb +++ b/test/test_redis_connection.rb @@ -10,6 +10,22 @@ class TestRedisConnection < Minitest::Test assert_equal Redis, pool.checkout.class end + describe "network_timeout" do + it "sets a custom network_timeout if specified" do + pool = Sidekiq::RedisConnection.create(:network_timeout => 8) + redis = pool.checkout + + assert_equal 8, redis.client.timeout + end + + it "uses the default network_timeout if none specified" do + pool = Sidekiq::RedisConnection.create + redis = pool.checkout + + assert_equal 5, redis.client.timeout + end + end + describe "namespace" do it "uses a given :namespace" do pool = Sidekiq::RedisConnection.create(:namespace => "xxx") @@ -23,9 +39,9 @@ class TestRedisConnection < Minitest::Test end end - describe "timeout" do + describe "pool_timeout" do it "uses a given :timeout over the default of 1" do - pool = Sidekiq::RedisConnection.create(:timeout => 5) + pool = Sidekiq::RedisConnection.create(:pool_timeout => 5) assert_equal 5, pool.instance_eval{ @timeout } end