don't mark redis readonly errors (#489)

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+* Redis Readonly errors won't trigger semian open circuit. (#489)
+
 # v0.18.1
 
 * Remove dependency on activerecord-trilogy-adapter in Trilogy adapter for Semian. (#486)

lib/semian/redis.rb

@@ -32,6 +32,14 @@ def marks_semian_circuits?
     end
   end
 
+  class ReadOnlyError < Redis::CommandError
+    # A ReadOnlyError is a fast failure and we don't want to track these errors so that we can reconnect
+    # to the new primary ASAP
+    def marks_semian_circuits?
+      false
+    end
+  end
+
   ResourceBusyError = Class.new(SemianError)
   CircuitOpenError = Class.new(SemianError)
   ResolveError = Class.new(SemianError)

lib/semian/redis/v5.rb

@@ -6,6 +6,14 @@ class Redis
   BaseConnectionError.include(::Semian::AdapterError)
   OutOfMemoryError.include(::Semian::AdapterError)
 
+  class ReadOnlyError < Redis::BaseConnectionError
+    # A ReadOnlyError is a fast failure and we don't want to track these errors so that we can reconnect
+    # to the new primary ASAP
+    def marks_semian_circuits?
+      false
+    end
+  end
+
   class SemianError < BaseConnectionError
     def initialize(semian_identifier, *args)
       super(*args)

lib/semian/redis_client.rb

@@ -14,6 +14,14 @@ def marks_semian_circuits?
 
   OutOfMemoryError.include(::Semian::AdapterError)
 
+  class ReadOnlyError < RedisClient::ConnectionError
+    # A ReadOnlyError is a fast failure and we don't want to track these errors so that we can reconnect
+    # to the new primary ASAP
+    def marks_semian_circuits?
+      false
+    end
+  end
+
   class SemianError < ConnectionError
     def initialize(semian_identifier, *args)
       super(*args)

test/adapters/redis_client_test.rb

@@ -145,6 +145,22 @@ def test_redis_connection_errors_are_tagged_with_the_resource_identifier
     end
   end
 
+  def test_readonly_errors_does_not_open_the_circuit
+    client = connect_to_redis!
+
+    with_readonly_mode(client) do
+      ERROR_THRESHOLD.times do
+        assert_raises(RedisClient::ReadOnlyError) do
+          client.call("set", "foo", "bar")
+        end
+      end
+
+      assert_raises(RedisClient::ReadOnlyError) do
+        client.call("set", "foo", "bar")
+      end
+    end
+  end
+
   def test_other_redis_errors_are_not_tagged_with_the_resource_identifier
     client = connect_to_redis!
     client.call("set", "foo", "bar")
@@ -379,6 +395,13 @@ def new_client(**options)
     new_config(**options).new_client
   end
 
+  def with_readonly_mode(client)
+    client.call("replicaof", "localhost", "6379")
+    yield
+  ensure
+    client.call("replicaof", "NO", "ONE")
+  end
+
   def new_config(**options)
     options[:host] = SemianConfig["toxiproxy_upstream_host"] if options[:host].nil?
     semian_options = SEMIAN_OPTIONS.merge(options.delete(:semian) || {})

test/adapters/redis_test.rb

@@ -448,6 +448,24 @@ def test_circuit_breaker_on_query
     end
   end
 
+  def test_readonly_errors_does_not_open_the_circuit
+    client = connect_to_redis!
+
+    with_readonly_mode(client) do
+      ERROR_THRESHOLD.times do
+        assert_raises(::Redis::ReadOnlyError, ::Redis::CommandError) do
+          client.set("foo", "bar")
+        end
+      end
+
+      error = assert_raises(::Redis::ReadOnlyError, ::Redis::CommandError) do
+        client.set("foo", "bar")
+      end
+
+      assert_match("READONLY You can't write against a read only replica.", error.message)
+    end
+  end
+
   private
 
   def new_redis(options = {})
@@ -489,6 +507,13 @@ def with_maxmemory(bytes)
     end
   end
 
+  def with_readonly_mode(client)
+    client.replicaof("localhost", "6379")
+    yield
+  ensure
+    client.replicaof("NO", "ONE")
+  end
+
   def redis_timeout_ms
     @redis_timeout_ms ||= (REDIS_TIMEOUT * 1000).to_i
   end