Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SentinelsClient: Use the given protocol to connect to sentinels #48

Merged
merged 1 commit into from
Aug 15, 2024
Merged

SentinelsClient: Use the given protocol to connect to sentinels #48

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/async/redis/sentinels.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def resolve_address

def resolve_master
@sentinel_endpoints.each do |sentinel_endpoint|
client = Client.new(sentinel_endpoint)
client = Client.new(sentinel_endpoint, protocol: @protocol)

begin
address = client.call('sentinel', 'get-master-addr-by-name', @master_name)
Expand All @@ -66,7 +66,7 @@ def resolve_master

def resolve_slave
@sentinel_endpoints.each do |sentinel_endpoint|
client = Client.new(sentinel_endpoint)
client = Client.new(sentinel_endpoint, protocol: @protocol)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be testable in some way? Can you think of a test that could check that this was the passed protocol?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this in Fedora following the instructions from #29.

Launch Redis master and replica as containers:

Master:

podman run --rm --net=host redis --port 6379 --requirepass p4ssW0rd --masterauth p4ssW0rd

Slave:

podman run --rm --net=host redis --port 6380 --requirepass p4ssW0rd --masterauth p4ssW0rd --slaveof localhost 6379

Create three sentinel.conf files with the next content:

port 26379
sentinel monitor redis-master 127.0.0.1 6379 2
sentinel down-after-milliseconds redis-master 5000
sentinel failover-timeout redis-master 60000
requirepass "p4ssW0rd"
sentinel auth-pass redis-master p4ssW0rd

But with a different port for each file: 26379, 26380, 26381.

Then launch the sentinels:

podman run --rm --net=host -v ./sentinel1:/data:z redis /data/sentinel.conf --port 26379 --sentinel

podman run --rm --net=host -v ./sentinel2:/data:z redis /data/sentinel.conf --port 26380 --sentinel

podman run --rm --net=host -v ./sentinel3:/data:z redis /data/sentinel.conf --port 26381 --sentinel

I created this small script to test the PR. Comment/uncomment the proper gemfile line to see the difference between master and this PR

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem 'async-redis'
  #gem 'async-redis', git: 'https://github.com/jlledom/async-redis', branch: 'sentinel-client-protocol'
end

class AuthenticatedRESP2
  def initialize(password)
    @password = password
  end

  def client(stream)
    client = Async::Redis::Protocol::RESP2.client(stream)

    client.write_request(["AUTH", *@password])
    client.read_response # Ignore response.

    client
  end
end

def main
  Async do
    sentinels = [{ host: "localhost", port: 26379 },
                 { host: "localhost", port: 26380 },
                 { host: "localhost", port: 26381 }]

    protocol = AuthenticatedRESP2.new('p4ssW0rd')

    client = Async::Redis::SentinelsClient.new('redis-master', sentinels, :master, protocol)

    while true
      begin
        sleep 1
        result = client.ping 'test'
        puts result
      rescue StandardError => e
        puts e.message
        retry
      end
    end
  end
end

main


begin
reply = client.call('sentinel', 'slaves', @master_name)
Expand Down
Loading