diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7528a50..2d04ac8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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' diff --git a/lib/faulty/storage/redis.rb b/lib/faulty/storage/redis.rb index bdd62c3..3b96c91 100644 --- a/lib/faulty/storage/redis.rb +++ b/lib/faulty/storage/redis.rb @@ -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`. @@ -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) @@ -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? @@ -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 diff --git a/spec/storage/redis_spec.rb b/spec/storage/redis_spec.rb index b46d965..002dacd 100644 --- a/spec/storage/redis_spec.rb +++ b/spec/storage/redis_spec.rb @@ -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 } @@ -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 @@ -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