lib/redis_failover/client.rb in redis_failover-0.3.0 vs lib/redis_failover/client.rb in redis_failover-0.4.0

@@ -7,11 +7,10 @@
     include Util
 
     RETRY_WAIT_TIME = 3
     REDIS_ERRORS = Errno.constants.map { |c| Errno.const_get(c) }.freeze
     REDIS_READ_OPS = Set[
-      :dbsize,
       :echo,
       :exists,
       :get,
       :getbit,
       :getrange,
@@ -20,30 +19,25 @@
       :hgetall,
       :hkeys,
       :hlen,
       :hmget,
       :hvals,
-      :info,
       :keys,
-      :lastsave,
       :lindex,
       :llen,
       :lrange,
       :mapped_hmget,
       :mapped_mget,
       :mget,
-      :ping,
       :scard,
       :sdiff,
-      :select,
       :sinter,
       :sismember,
       :smembers,
       :srandmember,
       :strlen,
       :sunion,
-      :ttl,
       :type,
       :zcard,
       :zcount,
       :zrange,
       :zrangebyscore,
@@ -52,10 +46,16 @@
       :zrevrangebyscore,
       :zrevrank,
       :zscore
     ].freeze
 
+    UNSUPPORTED_OPS = Set[
+      :select,
+      :ttl,
+      :dbsize,
+    ].freeze
+
     # Performance optimization: to avoid unnecessary method_missing calls,
     # we proactively define methods that dispatch to the underlying redis
     # calls.
     Redis.public_instance_methods(false).each do |method|
       define_method(method) do |*args, &block|
@@ -86,11 +86,10 @@
       @retry = options[:retry_failure] || true
       @max_retries = @retry ? options.fetch(:max_retries, 3) : 0
       @server_url = "http://#{options[:host]}:#{options[:port]}/redis_servers"
       @master = nil
       @slaves = []
-      @lock = Mutex.new
       build_clients
       start_background_monitor
     end
 
     def method_missing(method, *args, &block)
@@ -115,10 +114,11 @@
     def redis_operation?(method)
       Redis.public_instance_methods(false).include?(method)
     end
 
     def dispatch(method, *args, &block)
+      verify_supported!(method)
       tries = 0
 
       begin
         if REDIS_READ_OPS.include?(method)
           # send read operations to a slave
@@ -157,36 +157,34 @@
       end
       master
     end
 
     def build_clients
-      @lock.synchronize do
-        tries = 0
+      tries = 0
 
-        begin
-          logger.info('Checking for new redis nodes.')
-          nodes = fetch_nodes
-          return unless nodes_changed?(nodes)
+      begin
+        logger.info('Checking for new redis nodes.')
+        nodes = fetch_nodes
+        return unless nodes_changed?(nodes)
 
-          logger.info('Node change detected, rebuilding clients.')
-          master = new_clients_for(nodes[:master]).first if nodes[:master]
-          slaves = new_clients_for(*nodes[:slaves])
+        logger.info('Node change detected, rebuilding clients.')
+        master = new_clients_for(nodes[:master]).first if nodes[:master]
+        slaves = new_clients_for(*nodes[:slaves])
 
-          # once clients are successfully created, swap the references
-          @master = master
-          @slaves = slaves
-        rescue => ex
-          logger.error("Failed to fetch nodes from #{@server_url} - #{ex.message}")
-          logger.error(ex.backtrace.join("\n"))
+        # once clients are successfully created, swap the references
+        @master = master
+        @slaves = slaves
+      rescue => ex
+        logger.error("Failed to fetch nodes 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)
+        if tries < @max_retries
+          tries += 1
+          sleep(RETRY_WAIT_TIME) && retry
         end
+
+        raise FailoverServerUnavailableError.new(@server_url)
       end
     end
 
     def fetch_nodes
       open(@server_url) do |io|
@@ -220,9 +218,15 @@
       current_role = node.info['role']
       if current_role.to_sym != role
         raise InvalidNodeRoleError.new(address_for(node), role, current_role)
       end
       role
+    end
+
+    def verify_supported!(method)
+      if UNSUPPORTED_OPS.include?(method)
+        raise UnsupportedOperationError.new(method)
+      end
     end
 
     def addresses_for(nodes)
       nodes.map { |node| address_for(node) }
     end