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.

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

Sentinel role support #114

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Sentinel role support #114

wants to merge 12 commits into from

Conversation

melo
Copy link
Member

@melo melo commented Jul 11, 2015

This is still work in progress. It works, but there is stuff missing.

In particular:

  • tests: not sure how to test this stuff yet;
  • corner case of wrong role detected: we connected during a demotion or promotion of the server we connected to, and the role no longer matches what we expected - we need a clean way to go through the reconnection process.

The first item should be easy, I just need some time with the sentinel tests to understand them well.

The second, I think the best bet is make the code path for Sentinel-based connections to be the same as the "reconnect on" path. We get reconnects for free if we have the wrong server Role, and the build sock stuff already takes care of going through Sentinel to pick up a new server.

Anyway, please review…

Test script during development:

#!/usr/bin/env perl

use 5.014;
use warnings;
use lib 'lib';
use Redis::Sentinel;
use Data::Dumper;

my $r = Redis->new(
  sentinels => ['127.0.0.1:2001', '127.0.0.1:2002', '127.0.0.1:2003'],
  service   => 't',
  role      => 'slave',
);

my $i = $r->info('replication');
warn "from $r->{server}: " . Dumper($i);

melo added 12 commits July 11, 2015 19:24
* add brief introduction to new();
* explain the order in which the multiple connection methods will be
  evaluated;
* break the server section into one per connection method.

Signed-off-by: Pedro Melo <[email protected]>
allows sentinel-controlled connections to a slave of the service.

Signed-off-by: Pedro Melo <[email protected]>
Allows user to pick and choose the type of server to connect to, master or
slave.

Corner case where we end up connected to the wrong type of server because
we are in the middle of a failover operation is *not* handled yet...

Signed-off-by: Pedro Melo <[email protected]>
Signed-off-by: Pedro Melo <[email protected]>
Signed-off-by: Pedro Melo <[email protected]>
@melo
Copy link
Member Author

melo commented Jul 11, 2015

Added some tests, I'll see the reconnect tomorrow...

@melo
Copy link
Member Author

melo commented Aug 10, 2015

@dams care to review this? :)

@dams
Copy link
Member

dams commented Aug 10, 2015

at first glance it looks good to me, I'll have a detailed look a bit later. Thanks for the work !

@dams dams force-pushed the master branch 2 times, most recently from e5ccb43 to 0dfcf02 Compare August 17, 2020 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants