Skip to content

Commit

Permalink
(POOLER-184) Pool manager retry and exit on failure
Browse files Browse the repository at this point in the history
Adding a reconnect retry for redis, which by default would retry 10 times,
for a total wait time of ~80 seconds
  • Loading branch information
Samuel Beaulieu committed Sep 2, 2020
1 parent bdf77a9 commit a35d66c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
3 changes: 2 additions & 1 deletion bin/vmpooler
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ redis_port = config[:redis]['port']
redis_password = config[:redis]['password']
redis_connection_pool_size = config[:redis]['connection_pool_size']
redis_connection_pool_timeout = config[:redis]['connection_pool_timeout']
redis_reconnect_attempts = config[:redis]['reconnect_attempts'] || 10
logger_file = config[:config]['logfile']

logger = Vmpooler::Logger.new logger_file
Expand Down Expand Up @@ -43,7 +44,7 @@ if torun.include? :manager
Vmpooler::PoolManager.new(
config,
logger,
Vmpooler.redis_connection_pool(redis_host, redis_port, redis_password, redis_connection_pool_size, redis_connection_pool_timeout, metrics),
Vmpooler.redis_connection_pool(redis_host, redis_port, redis_password, redis_connection_pool_size, redis_connection_pool_timeout, metrics, redis_reconnect_attempts),
metrics
).execute!
end
Expand Down
9 changes: 5 additions & 4 deletions lib/vmpooler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def self.load_pools_from_redis(redis)
pools
end

def self.redis_connection_pool(host, port, password, size, timeout, metrics)
def self.redis_connection_pool(host, port, password, size, timeout, metrics, redis_reconnect_attempts = 0)
Vmpooler::PoolManager::GenericConnectionPool.new(
metrics: metrics,
connpool_type: 'redis_connection_pool',
Expand All @@ -173,13 +173,14 @@ def self.redis_connection_pool(host, port, password, size, timeout, metrics)
timeout: timeout
) do
connection = Concurrent::Hash.new
redis = new_redis(host, port, password)
redis = new_redis(host, port, password, redis_reconnect_attempts)
connection['connection'] = redis
end
end

def self.new_redis(host = 'localhost', port = nil, password = nil)
Redis.new(host: host, port: port, password: password)
def self.new_redis(host = 'localhost', port = nil, password = nil, redis_reconnect_attempts = 10)
Redis.new(host: host, port: port, password: password, reconnect_attempts: redis_reconnect_attempts, reconnect_delay: 1.5,
reconnect_delay_max: 10.0)
end

def self.pools(conf)
Expand Down
7 changes: 7 additions & 0 deletions spec/unit/generic_connection_pool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
connection: 'connection'
}}

it 'should error out when connection pool could not establish no nothing' do
newsub = Vmpooler.redis_connection_pool("foo,bar", "1234", "fuba", 1, 1, metrics, 0)
expect { newsub.with_metrics do |conn_pool_object|
conn_pool_object.srem('foo', "bar")
end }.to raise_error Redis::CannotConnectError
end

it 'should return a connection object when grabbing one from the pool' do
subject.with_metrics do |conn_pool_object|
expect(conn_pool_object).to be(connection_object)
Expand Down

0 comments on commit a35d66c

Please sign in to comment.