lib/redis_failover/client.rb in redis_failover-0.1.1 vs lib/redis_failover/client.rb in redis_failover-0.2.0

@@ -4,10 +4,11 @@
 module RedisFailover
   # Redis failover-aware client.
   class Client
     include Util
 
+    RETRY_WAIT_TIME = 3
     REDIS_ERRORS = Errno.constants.map { |c| Errno.const_get(c) }.freeze
     REDIS_READ_OPS = Set[
       :dbsize,
       :echo,
       :exists,
@@ -81,12 +82,12 @@
 
       Util.logger = options[:logger] if options[:logger]
       @namespace = options[:namespace]
       @password = options[:password]
       @retry = options[:retry_failure] || true
-      @max_retries = options[:max_retries] || 3
-      @registry_url = "http://#{options[:host]}:#{options[:port]}/redis_servers"
+      @max_retries = @retry ? options.fetch(:max_retries, 3) : 0
+      @server_url = "http://#{options[:host]}:#{options[:port]}/redis_servers"
       @redis_servers = nil
       @master = nil
       @slaves = []
       @lock = Mutex.new
       build_clients
@@ -115,61 +116,78 @@
       Redis.public_instance_methods(false).include?(method)
     end
 
     def dispatch(method, *args, &block)
       tries = 0
+
       begin
         if REDIS_READ_OPS.include?(method)
           # send read operations to a slave
           slave.send(method, *args, &block)
         else
           # direct everything else to master
           master.send(method, *args, &block)
         end
-      rescue NoMasterError, *REDIS_ERRORS
+      rescue Error, *REDIS_ERRORS
         logger.error("No suitable node available for operation `#{method}.`")
-        sleep(3)
         build_clients
 
-        if @retry && tries < @max_retries
+        if tries < @max_retries
           tries += 1
-          retry
+          sleep(RETRY_WAIT_TIME) && retry
         end
 
         raise
       end
     end
 
     def master
-      @master or raise NoMasterError
+      if @master
+        verify_role!(@master, :master)
+        return @master
+      end
+      raise NoMasterError
     end
 
     def slave
       # pick a slave, if none available fallback to master
-      @slaves.sample || master
+      if slave = @slaves.sample
+        verify_role!(slave, :slave)
+        return slave
+      end
+      master
     end
 
     def build_clients
       @lock.synchronize do
+        tries = 0
+
         begin
           logger.info('Attempting to fetch nodes and build redis clients.')
           servers = fetch_redis_servers
           master = new_clients_for(servers[:master]).first if servers[:master]
           slaves = new_clients_for(*servers[:slaves])
 
           # once clients are successfully created, swap the references
           @master = master
           @slaves = slaves
         rescue => ex
-          logger.error("Failed to fetch servers from #{@registry_url} - #{ex.message}")
+          logger.error("Failed to fetch servers from #{@server_url} - #{ex.message}")
           logger.error(ex.backtrace.join("\n"))
+
+          if tries < @max_retries
+            tries += 1
+            sleep(RETRY_WAIT_TIME) && retry
+          end
+
+          raise FailoverServerUnreachableError.new(@server_url)
         end
       end
     end
 
     def fetch_redis_servers
-      open(@registry_url) do |io|
+      open(@server_url) do |io|
         servers = symbolize_keys(MultiJson.decode(io))
         logger.info("Fetched servers: #{servers}")
         servers
       end
     end
@@ -185,14 +203,26 @@
       end
     end
 
     def master_info
       return "none" unless @master
-      "#{@master.client.host}:#{@master.client.port}"
+      name_for(@master)
     end
 
     def slaves_info
       return "none" if @slaves.empty?
-      @slaves.map { |s| "#{s.client.host}:#{s.client.port}" }.join(', ')
+      @slaves.map { |slave| name_for(slave) }.join(', ')
+    end
+
+    def verify_role!(node, role)
+      current_role = node.info['role']
+      if current_role.to_sym != role
+        raise InvalidNodeRoleError.new(name_for(node), role, current_role)
+      end
+      role
+    end
+
+    def name_for(node)
+      "#{node.client.host}:#{node.client.port}"
     end
   end
 end