From 3893355cf84279e894eb10f7f353bcc7e8ed8df4 Mon Sep 17 00:00:00 2001 From: Chris Heald Date: Fri, 26 Feb 2016 19:58:10 -0700 Subject: [PATCH 1/3] Add the :nearest_slave role for Sentinel mode This will cause the client to measure roundtrip latency to each slave and select the slave with the lowest latency. The intent for this is to enable sentinel-managed clusters of servers for which eventually-consistent reads are acceptable, but to maintain minimum latencies between any individual client-slave pair. The case I did this for is is shared web application caching across multiple datacenters, where you would not want Redis to connect to a slave in another datacenter, but you would want all datacenters to share a cache. Remove trailing comma from client creation options; should fix 1.8 builds If we can't get the role, use a translated role Ensure that ping test clients are always disconnected after use. Don't assume that a good slave was found. --- lib/redis/client.rb | 40 ++++++++++++++++++++++++++++++++++++--- test/sentinel_test.rb | 44 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) mode change 100644 => 100755 lib/redis/client.rb mode change 100644 => 100755 test/sentinel_test.rb diff --git a/lib/redis/client.rb b/lib/redis/client.rb old mode 100644 new mode 100755 index 5ef5fdb6e..e95839669 --- a/lib/redis/client.rb +++ b/lib/redis/client.rb @@ -529,6 +529,10 @@ def resolve def check(client); end class Sentinel < Connector + EXPECTED_ROLES = { + "nearest_slave" => "slave" + } + def initialize(options) super(options) @@ -547,12 +551,12 @@ def check(client) role = client.call([:role])[0] rescue Redis::CommandError # Assume the test is passed if we can't get a reply from ROLE... - role = @role + role = EXPECTED_ROLES.fetch(@role, @role) end - if role != @role + if role != EXPECTED_ROLES.fetch(@role, @role) client.disconnect - raise ConnectionError, "Instance role mismatch. Expected #{@role}, got #{role}." + raise ConnectionError, "Instance role mismatch. Expected #{EXPECTED_ROLES.fetch(@role, @role)}, got #{role}." end end @@ -562,6 +566,8 @@ def resolve resolve_master when "slave" resolve_slave + when "nearest_slave" + resolve_nearest_slave else raise ArgumentError, "Unknown instance role #{@role}" end @@ -622,6 +628,34 @@ def resolve_slave end end end + + def resolve_nearest_slave + sentinel_detect do |client| + if reply = client.call(["sentinel", "slaves", @master]) + ok_slaves = reply.map {|r| Hash[*r] }.select {|r| r["master-link-status"] == "ok" } + + ok_slaves.each do |slave| + client = Client.new @options.merge( + :host => slave["ip"], + :port => slave["port"], + :reconnect_attempts => 0 + ) + begin + client.call [:ping] + start = Time.now + client.call [:ping] + slave["response_time"] = (Time.now - start).to_f + ensure + client.disconnect + end + end + + slave = ok_slaves.sort_by {|slave| slave["response_time"] }.first + {:host => slave.fetch("ip"), :port => slave.fetch("port")} if slave + end + end + end + end end end diff --git a/test/sentinel_test.rb b/test/sentinel_test.rb old mode 100644 new mode 100755 index 2af3f3934..cce153e5e --- a/test/sentinel_test.rb +++ b/test/sentinel_test.rb @@ -377,4 +377,48 @@ def test_sentinel_with_string_option_keys assert_equal [%w[get-master-addr-by-name master1]], commands end + + def test_sentinel_nearest_slave + sentinels = [{:host => "127.0.0.1", :port => 26381}] + + master = { :role => lambda { ["master"] } } + s1 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["1"] }, :ping => lambda { ["OK"] } } + s2 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["2"] }, :ping => lambda { sleep 0.1; ["OK"] } } + s3 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["3"] }, :ping => lambda { sleep 0.2; ["OK"] } } + + 5.times do + RedisMock.start(master) do |master_port| + RedisMock.start(s1) do |s1_port| + RedisMock.start(s2) do |s2_port| + RedisMock.start(s3) do |s3_port| + + sentinel = lambda do |port| + { + :sentinel => lambda do |command, *args| + case command + when "slaves" + [ + %W[master-link-status down ip 127.0.0.1 port #{s1_port}], + %W[master-link-status ok ip 127.0.0.1 port #{s2_port}], + %W[master-link-status ok ip 127.0.0.1 port #{s3_port}] + ].shuffle + else + ["127.0.0.1", port.to_s] + end + end + } + end + + RedisMock.start(sentinel.call(master_port)) do |sen_port| + sentinels[0][:port] = sen_port + redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :nearest_slave) + assert_equal redis.slave_id, ["2"] + end + end + end + end + end + end + + end end From 0e52d43b8f1ccb7a850f7d4a6582c13911cc0fa3 Mon Sep 17 00:00:00 2001 From: Chris Heald Date: Thu, 21 Apr 2016 00:53:37 -0700 Subject: [PATCH 2/3] Add the :nearest mode, which selects the closest node by ping, regardless of role --- lib/redis/client.rb | 65 +++++++++++++++++++++++++++++-------------- test/sentinel_test.rb | 17 +++++------ 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/lib/redis/client.rb b/lib/redis/client.rb index e95839669..d05386af9 100755 --- a/lib/redis/client.rb +++ b/lib/redis/client.rb @@ -530,7 +530,8 @@ def check(client); end class Sentinel < Connector EXPECTED_ROLES = { - "nearest_slave" => "slave" + "nearest_slave" => "slave", + "nearest" => "any" } def initialize(options) @@ -547,14 +548,15 @@ def check(client) # Check the instance is really of the role we are looking for. # We can't assume the command is supported since it was introduced # recently and this client should work with old stuff. + expected_role = EXPECTED_ROLES.fetch(@role, @role) begin role = client.call([:role])[0] rescue Redis::CommandError # Assume the test is passed if we can't get a reply from ROLE... - role = EXPECTED_ROLES.fetch(@role, @role) + role = expected_role end - if role != EXPECTED_ROLES.fetch(@role, @role) + if role != expected_role && "any" != expected_role client.disconnect raise ConnectionError, "Instance role mismatch. Expected #{EXPECTED_ROLES.fetch(@role, @role)}, got #{role}." end @@ -566,6 +568,8 @@ def resolve resolve_master when "slave" resolve_slave + when "nearest" + resolve_nearest when "nearest_slave" resolve_nearest_slave else @@ -629,30 +633,49 @@ def resolve_slave end end + def resolve_nearest + resolve_nearest_for [:master, :slaves] + end + def resolve_nearest_slave + resolve_nearest_for [:slaves] + end + + def resolve_nearest_for(types) sentinel_detect do |client| - if reply = client.call(["sentinel", "slaves", @master]) - ok_slaves = reply.map {|r| Hash[*r] }.select {|r| r["master-link-status"] == "ok" } - - ok_slaves.each do |slave| - client = Client.new @options.merge( - :host => slave["ip"], - :port => slave["port"], - :reconnect_attempts => 0 - ) - begin - client.call [:ping] - start = Time.now - client.call [:ping] - slave["response_time"] = (Time.now - start).to_f - ensure - client.disconnect + ok_nodes = [] + types.each do |type| + if reply = client.call(["sentinel", type, @master]) + reply = [reply] if type == :master + ok_nodes += reply.map {|r| Hash[*r] }.select do |r| + case type + when :master + r["role-reported"] == "master" + when :slaves + r["master-link-status"] == "ok" && !r.fetch("flags", "").match(/s_down|disconnected/) + end end end + end - slave = ok_slaves.sort_by {|slave| slave["response_time"] }.first - {:host => slave.fetch("ip"), :port => slave.fetch("port")} if slave + ok_nodes.each do |node| + client = Client.new @options.merge( + :host => node["ip"], + :port => node["port"], + :reconnect_attempts => 0 + ) + begin + client.call [:ping] + start = Time.now + client.call [:ping] + node["response_time"] = (Time.now - start).to_f + ensure + client.disconnect + end end + + node = ok_nodes.sort_by {|node| node["response_time"] }.first + {:host => node.fetch("ip"), :port => node.fetch("port")} if node end end diff --git a/test/sentinel_test.rb b/test/sentinel_test.rb index cce153e5e..8cf51bbb5 100755 --- a/test/sentinel_test.rb +++ b/test/sentinel_test.rb @@ -378,13 +378,13 @@ def test_sentinel_with_string_option_keys assert_equal [%w[get-master-addr-by-name master1]], commands end - def test_sentinel_nearest_slave + def test_sentinel_nearest sentinels = [{:host => "127.0.0.1", :port => 26381}] - master = { :role => lambda { ["master"] } } - s1 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["1"] }, :ping => lambda { ["OK"] } } - s2 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["2"] }, :ping => lambda { sleep 0.1; ["OK"] } } - s3 = { :role => lambda { ["slave"] }, :slave_id => lambda { ["3"] }, :ping => lambda { sleep 0.2; ["OK"] } } + master = { :role => lambda { ["master"] }, :node_id => lambda { ["master"] }, :ping => lambda { ["OK"] } } + s1 = { :role => lambda { ["slave"] }, :node_id => lambda { ["1"] }, :ping => lambda { sleep 0.1; ["OK"] } } + s2 = { :role => lambda { ["slave"] }, :node_id => lambda { ["2"] }, :ping => lambda { sleep 0.2; ["OK"] } } + s3 = { :role => lambda { ["slave"] }, :node_id => lambda { ["3"] }, :ping => lambda { sleep 0.3; ["OK"] } } 5.times do RedisMock.start(master) do |master_port| @@ -396,6 +396,8 @@ def test_sentinel_nearest_slave { :sentinel => lambda do |command, *args| case command + when "master" + %W[role-reported master ip 127.0.0.1 port #{master_port}] when "slaves" [ %W[master-link-status down ip 127.0.0.1 port #{s1_port}], @@ -411,14 +413,13 @@ def test_sentinel_nearest_slave RedisMock.start(sentinel.call(master_port)) do |sen_port| sentinels[0][:port] = sen_port - redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :nearest_slave) - assert_equal redis.slave_id, ["2"] + redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :nearest) + assert_equal ["master"], redis.node_id end end end end end end - end end From a49c23d35f990cc97d4e6fa06b23e6d0623f02f0 Mon Sep 17 00:00:00 2001 From: Chris Heald Date: Mon, 17 Aug 2020 10:25:25 -0700 Subject: [PATCH 3/3] Style fixes to make Rubocop happy --- lib/redis/client.rb | 40 ++++++++++++++++++++-------------------- test/sentinel_test.rb | 15 +++++++-------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/lib/redis/client.rb b/lib/redis/client.rb index d05386af9..e638bfa6c 100755 --- a/lib/redis/client.rb +++ b/lib/redis/client.rb @@ -532,7 +532,7 @@ class Sentinel < Connector EXPECTED_ROLES = { "nearest_slave" => "slave", "nearest" => "any" - } + }.freeze def initialize(options) super(options) @@ -556,9 +556,9 @@ def check(client) role = expected_role end - if role != expected_role && "any" != expected_role + if role != expected_role && expected_role != "any" client.disconnect - raise ConnectionError, "Instance role mismatch. Expected #{EXPECTED_ROLES.fetch(@role, @role)}, got #{role}." + raise ConnectionError, "Instance role mismatch. Expected #{expected_role}, got #{role}." end end @@ -634,35 +634,36 @@ def resolve_slave end def resolve_nearest - resolve_nearest_for [:master, :slaves] + resolve_nearest_for %I(master slaves) end def resolve_nearest_slave - resolve_nearest_for [:slaves] + resolve_nearest_for %I(slaves) end def resolve_nearest_for(types) sentinel_detect do |client| ok_nodes = [] types.each do |type| - if reply = client.call(["sentinel", type, @master]) - reply = [reply] if type == :master - ok_nodes += reply.map {|r| Hash[*r] }.select do |r| - case type - when :master - r["role-reported"] == "master" - when :slaves - r["master-link-status"] == "ok" && !r.fetch("flags", "").match(/s_down|disconnected/) - end + reply = client.call(["sentinel", type, @master]) + next unless reply + + reply = [reply] if type == :master + ok_nodes += reply.map { |r| Hash[*r] }.select do |r| + case type + when :master + r["role-reported"] == "master" + when :slaves + r["master-link-status"] == "ok" && !r.fetch("flags", "").match(/s_down|disconnected/) end end end ok_nodes.each do |node| client = Client.new @options.merge( - :host => node["ip"], - :port => node["port"], - :reconnect_attempts => 0 + host: node["ip"], + port: node["port"], + reconnect_attempts: 0 ) begin client.call [:ping] @@ -674,11 +675,10 @@ def resolve_nearest_for(types) end end - node = ok_nodes.sort_by {|node| node["response_time"] }.first - {:host => node.fetch("ip"), :port => node.fetch("port")} if node + node = ok_nodes.min_by { |n| n["response_time"] } + { host: node.fetch("ip"), port: node.fetch("port") } if node end end - end end end diff --git a/test/sentinel_test.rb b/test/sentinel_test.rb index 8cf51bbb5..774591434 100755 --- a/test/sentinel_test.rb +++ b/test/sentinel_test.rb @@ -379,22 +379,21 @@ def test_sentinel_with_string_option_keys end def test_sentinel_nearest - sentinels = [{:host => "127.0.0.1", :port => 26381}] + sentinels = [{ host: "127.0.0.1", port: 26_381 }] - master = { :role => lambda { ["master"] }, :node_id => lambda { ["master"] }, :ping => lambda { ["OK"] } } - s1 = { :role => lambda { ["slave"] }, :node_id => lambda { ["1"] }, :ping => lambda { sleep 0.1; ["OK"] } } - s2 = { :role => lambda { ["slave"] }, :node_id => lambda { ["2"] }, :ping => lambda { sleep 0.2; ["OK"] } } - s3 = { :role => lambda { ["slave"] }, :node_id => lambda { ["3"] }, :ping => lambda { sleep 0.3; ["OK"] } } + master = { role: -> { ["master"] }, node_id: -> { ["master"] }, ping: -> { ["OK"] } } + s1 = { role: -> { ["slave"] }, node_id: -> { ["1"] }, ping: -> { sleep 0.1; ["OK"] } } + s2 = { role: -> { ["slave"] }, node_id: -> { ["2"] }, ping: -> { sleep 0.2; ["OK"] } } + s3 = { role: -> { ["slave"] }, node_id: -> { ["3"] }, ping: -> { sleep 0.3; ["OK"] } } 5.times do RedisMock.start(master) do |master_port| RedisMock.start(s1) do |s1_port| RedisMock.start(s2) do |s2_port| RedisMock.start(s3) do |s3_port| - sentinel = lambda do |port| { - :sentinel => lambda do |command, *args| + sentinel: lambda do |command, *_args| case command when "master" %W[role-reported master ip 127.0.0.1 port #{master_port}] @@ -413,7 +412,7 @@ def test_sentinel_nearest RedisMock.start(sentinel.call(master_port)) do |sen_port| sentinels[0][:port] = sen_port - redis = Redis.new(:url => "redis://master1", :sentinels => sentinels, :role => :nearest) + redis = Redis.new(url: "redis://master1", sentinels: sentinels, role: :nearest) assert_equal ["master"], redis.node_id end end