Skip to content

Commit

Permalink
Add support for redis gem v5
Browse files Browse the repository at this point in the history
  • Loading branch information
justinhoward committed Apr 27, 2023
1 parent 13b0d9f commit 32d97e9
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ jobs:
- ruby: '2.7'
redis: '4'
search: ['opensearch-ruby:1.0.1', 'opensearchproject/opensearch:1.0.1']
# Redis 5
# Will add support after fixing patch
# - ruby: '2.7'
# redis: '5'
# search: [['opensearch-ruby:2.1.0', 'opensearchproject/opensearch:2.2.1']]
# Ruby 2.3 & Elasticsearch 7.13
- ruby: '2.3'
redis: '4'
Expand Down
18 changes: 12 additions & 6 deletions lib/faulty/storage/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Redis
# circuit state. Default `faulty`.
# @!attribute [r] key_separator
# @return [String] A string used to separate the parts of the Redis keys
# used to store circuit state. Defaulty `:`.
# used to store circuit state. Default `:`.
# @!attribute [r] max_sample_size
# @return [Integer] The number of cache run entries to keep in memory
# for each circuit. Default `100`.
Expand Down Expand Up @@ -122,7 +122,7 @@ def set_options(circuit, stored_options)
def entry(circuit, time, success, status)
key = entries_key(circuit.name)
result = pipe do |r|
r.sadd(list_key, circuit.name)
r.call([:sadd, list_key, circuit.name])
r.expire(list_key, options.circuit_ttl + options.list_granularity) if options.circuit_ttl
r.lpush(key, "#{time}#{ENTRY_SEPARATOR}#{success ? 1 : 0}")
r.ltrim(key, 0, options.max_sample_size - 1)
Expand Down Expand Up @@ -425,11 +425,16 @@ def check_client_options!
end

def check_redis_options!
ropts = redis { |r| r.instance_variable_get(:@client).options }
gte5 = ::Redis::VERSION.to_f >= 5
method = gte5 ? :config : :options
ropts = redis do |r|
r.instance_variable_get(:@client).public_send(method)
end

bad_timeouts = {}
%i[connect_timeout read_timeout write_timeout].each do |time_opt|
bad_timeouts[time_opt] = ropts[time_opt] if ropts[time_opt] > 2
value = gte5 ? ropts.public_send(time_opt) : ropts[time_opt]
bad_timeouts[time_opt] = value if value > 2
end

unless bad_timeouts.empty?
Expand All @@ -440,10 +445,11 @@ def check_redis_options!
MSG
end

if ropts[:reconnect_attempts] > 1
gt1_retry = gte5 ? ropts.retry_connecting?(1, nil) : ropts[:reconnect_attempts] > 1
if gt1_retry
warn <<~MSG
Faulty recommends setting Redis reconnect_attempts to <= 1 to
prevent cascading failures. Your setting is #{ropts[:reconnect_attempts]}
prevent cascading failures. Your setting is larger.
MSG
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/storage/redis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
end

context 'when Redis has high timeout' do
let(:client) { Redis.new(timeout: 5) }
let(:client) { Redis.new(timeout: 5.0) }

it 'prints timeout warning' do
timeouts = { connect_timeout: 5.0, read_timeout: 5.0, write_timeout: 5.0 }
Expand All @@ -63,10 +63,10 @@
end

context 'when Redis has high reconnect_attempts' do
let(:client) { Redis.new(timeout: 1, reconnect_attempts: 3) }
let(:client) { Redis.new(timeout: 1, reconnect_attempts: 2) }

it 'prints reconnect_attempts warning' do
expect { storage }.to output(/Your setting is 3/).to_stderr
expect { storage }.to output(/Your setting is larger/).to_stderr
end
end

Expand All @@ -82,7 +82,7 @@

context 'when ConnectionPool Redis client has high timeout' do
let(:client) do
ConnectionPool.new(timeout: 1) { Redis.new(timeout: 7) }
ConnectionPool.new(timeout: 1) { Redis.new(timeout: 7.0) }
end

it 'prints Redis timeout warning' do
Expand Down

0 comments on commit 32d97e9

Please sign in to comment.