Merge pull request #505 from redis/sentinel-tests

Sentinel tests + Hiredis fix

Author: djanowski

lib/redis/client.rb

@@ -317,9 +317,9 @@ def establish_connection
       server = @connector.resolve.dup
 
       @options[:host] = server[:host]
-      @options[:port] = server[:port]
+      @options[:port] = Integer(server[:port]) if server.include?(:port)
 
-      @connection = @options[:driver].connect(server)
+      @connection = @options[:driver].connect(@options)
       @pending_reads = 0
     rescue TimeoutError,
            Errno::ECONNREFUSED,
@@ -366,6 +366,8 @@ def ensure_connected
     end
 
     def _parse_options(options)
+      return options if options[:_parsed]
+
       defaults = DEFAULTS.dup
       options = options.dup
 
@@ -450,6 +452,8 @@ def _parse_options(options)
         end
       end
 
+      options[:_parsed] = true
+
       options
     end
 
@@ -470,7 +474,7 @@ def _parse_driver(driver)
 
     class Connector
       def initialize(options)
-        @options = options
+        @options = options.dup
       end
 
       def resolve
@@ -484,9 +488,12 @@ class Sentinel < Connector
         def initialize(options)
           super(options)
 
-          @sentinels = options.fetch(:sentinels).dup
-          @role = options.fetch(:role, "master").to_s
-          @master = options[:host]
+          @options[:password] = DEFAULTS.fetch(:password)
+          @options[:db] = DEFAULTS.fetch(:db)
+
+          @sentinels = @options.delete(:sentinels).dup
+          @role = @options.fetch(:role, "master").to_s
+          @master = @options[:host]
         end
 
         def check(client)
@@ -501,7 +508,7 @@ def check(client)
           end
 
           if role != @role
-            disconnect
+            client.disconnect
             raise ConnectionError, "Instance role mismatch. Expected #{@role}, got #{role}."
           end
         end
@@ -521,7 +528,10 @@ def resolve
 
         def sentinel_detect
           @sentinels.each do |sentinel|
-            client = Client.new(:host => sentinel[:host], :port => sentinel[:port], :timeout => 0.3)
+            client = Client.new(@options.merge({
+              :host => sentinel[:host],
+              :port => sentinel[:port]
+            }))
 
             begin
               if result = yield(client)

lib/redis/connection/hiredis.rb

@@ -9,11 +9,12 @@ class Hiredis
 
       def self.connect(config)
         connection = ::Hiredis::Connection.new
+        connect_timeout = (config.fetch(:connect_timeout, 0) * 1_000_000).to_i
 
         if config[:scheme] == "unix"
-          connection.connect_unix(config[:path], Integer(config[:connect_timeout] * 1_000_000))
+          connection.connect_unix(config[:path], connect_timeout)
         else
-          connection.connect(config[:host], config[:port], Integer(config[:connect_timeout] * 1_000_000))
+          connection.connect(config[:host], config[:port], connect_timeout)
         end
 
         instance = new(connection)

test/sentinel_test.rb

@@ -0,0 +1,186 @@
+# encoding: UTF-8
+
+require File.expand_path("helper", File.dirname(__FILE__))
+
+class SentinalTest < Test::Unit::TestCase
+
+  include Helper::Client
+
+  def test_sentinel_connection
+    sentinels = [{:host => "127.0.0.1", :port => 26381},
+                 {:host => "127.0.0.1", :port => 26382}]
+
+    commands = {
+      :s1 => [],
+      :s2 => [],
+    }
+
+    handler = lambda do |id|
+      {
+        :sentinel => lambda do |command, *args|
+          commands[id] << [command, *args]
+          ["127.0.0.1", "6381"]
+        end
+      }
+    end
+
+    RedisMock.start(handler.call(:s1), {}, 26381) do
+      RedisMock.start(handler.call(:s2), {}, 26382) do
+        redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :master)
+
+        assert redis.ping
+      end
+    end
+
+    assert_equal commands[:s1], [%w[get-master-addr-by-name master1]]
+    assert_equal commands[:s2], []
+  end
+
+  def test_sentinel_failover
+    sentinels = [{:host => "127.0.0.1", :port => 26381},
+                 {:host => "127.0.0.1", :port => 26382}]
+
+    commands = {
+      :s1 => [],
+      :s2 => [],
+    }
+
+    s1 = {
+      :sentinel => lambda do |command, *args|
+        commands[:s1] << [command, *args]
+        "$-1" # Nil
+      end
+    }
+
+    s2 = {
+      :sentinel => lambda do |command, *args|
+        commands[:s2] << [command, *args]
+        ["127.0.0.1", "6381"]
+      end
+    }
+
+    RedisMock.start(s1, {}, 26381) do
+      RedisMock.start(s2, {}, 26382) do
+        redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :master)
+
+        assert redis.ping
+      end
+    end
+
+    assert_equal commands[:s1], [%w[get-master-addr-by-name master1]]
+    assert_equal commands[:s2], [%w[get-master-addr-by-name master1]]
+  end
+
+  def test_sentinel_failover_prioritize_healthy_sentinel
+    sentinels = [{:host => "127.0.0.1", :port => 26381},
+                 {:host => "127.0.0.1", :port => 26382}]
+
+    commands = {
+      :s1 => [],
+      :s2 => [],
+    }
+
+    s1 = {
+      :sentinel => lambda do |command, *args|
+        commands[:s1] << [command, *args]
+        "$-1" # Nil
+      end
+    }
+
+    s2 = {
+      :sentinel => lambda do |command, *args|
+        commands[:s2] << [command, *args]
+        ["127.0.0.1", "6381"]
+      end
+    }
+
+    RedisMock.start(s1, {}, 26381) do
+      RedisMock.start(s2, {}, 26382) do
+        redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :master)
+
+        assert redis.ping
+
+        redis.quit
+
+        assert redis.ping
+      end
+    end
+
+    assert_equal commands[:s1], [%w[get-master-addr-by-name master1]]
+    assert_equal commands[:s2], [%w[get-master-addr-by-name master1], %w[get-master-addr-by-name master1]]
+  end
+
+  def test_sentinel_with_non_sentinel_options
+    sentinels = [{:host => "127.0.0.1", :port => 26381}]
+
+    commands = {
+      :s1 => [],
+      :m1 => []
+    }
+
+    sentinel = {
+      :auth => lambda do |pass|
+        commands[:s1] << ["auth", pass]
+        "-ERR unknown command 'auth'"
+      end,
+      :select => lambda do |db|
+        commands[:s1] << ["select", db]
+        "-ERR unknown command 'select'"
+      end,
+      :sentinel => lambda do |command, *args|
+        commands[:s1] << [command, *args]
+        ["127.0.0.1", "6382"]
+      end
+    }
+
+    master = {
+      :auth => lambda do |pass|
+        commands[:m1] << ["auth", pass]
+        "+OK"
+      end,
+      :role => lambda do
+        commands[:m1] << ["role"]
+        ["master"]
+      end
+    }
+
+    RedisMock.start(master, {}, 6382) do
+      RedisMock.start(sentinel, {}, 26381) do
+        redis = Redis.new(:url => "redis://:foo@master1/15", :sentinels => sentinels, :role => :master)
+
+        assert redis.ping
+      end
+    end
+
+    assert_equal [%w[get-master-addr-by-name master1]], commands[:s1]
+    assert_equal [%w[auth foo], %w[role]], commands[:m1]
+  end
+
+  def test_sentinel_role_mismatch
+    sentinels = [{:host => "127.0.0.1", :port => 26381}]
+
+    sentinel = {
+      :sentinel => lambda do |command, *args|
+        ["127.0.0.1", "6382"]
+      end
+    }
+
+    master = {
+      :role => lambda do
+        ["slave"]
+      end
+    }
+
+    ex = assert_raise(Redis::ConnectionError) do
+      RedisMock.start(master, {}, 6382) do
+        RedisMock.start(sentinel, {}, 26381) do
+          redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :master)
+
+          assert redis.ping
+        end
+      end
+    end
+
+    assert_match /Instance role mismatch/, ex.message
+  end
+end

test/support/redis_mock.rb

@@ -59,13 +59,13 @@ def run
   #     # Every connection will be closed immediately
   #   end
   #
-  def self.start_with_handler(blk, options = {})
-    server = Server.new(MOCK_PORT, options)
+  def self.start_with_handler(blk, options = {}, port = MOCK_PORT)
+    server = Server.new(port, options)
 
     begin
       server.start(&blk)
 
-      yield(MOCK_PORT)
+      yield(port)
 
     ensure
       server.shutdown
@@ -81,7 +81,7 @@ def self.start_with_handler(blk, options = {})
   #     assert_equal "PONG", Redis.new(:port => MOCK_PORT).ping
   #   end
   #
-  def self.start(commands, options = {}, &blk)
+  def self.start(commands, options = {}, port = MOCK_PORT, &blk)
     handler = lambda do |session|
       while line = session.gets
         argv = Array.new(line[1..-3].to_i) do
@@ -116,6 +116,6 @@ def self.start(commands, options = {}, &blk)
       end
     end
 
-    start_with_handler(handler, options, &blk)
+    start_with_handler(handler, options, port, &blk)
   end
 end