Giter Site home page Giter Site logo

Comments (18)

nrk avatar nrk commented on June 2, 2024

I have an overall idea of how this could be implemented with the current master branch of Predis, but I think there are a couple of tricky parts that should be cleared first.

How should be handled a scenario in which we start by reading stuff (e.g. multiple GET commands) and then we send a command that writes data (SET)? Our virtual connection could easily switch internally from the selected slave to the master connection, but what if this happens inside a MULTI / EXEC transaction? It's probably better to disallow MULTI / EXEC when using this connection abstraction, and manually get the connection to the master server out of the virtual connection to be able to use transactions.

In general, some commands should be disabled when using this approach (e.g. using SAVE, BGSAVE, SLAVEOF, etc. should be done by selecting explicitly a server) while others should be analyzed with more care (SORT with the STORE parameter translates to a write operation, otherwise it's a read operation). I'm not sure about the PUB/SUB related commands since I don't know how they behave with master/slave replication.

from predis.

Seldaek avatar Seldaek commented on June 2, 2024

Well actually I think your first issue is a non-issue, because you never do a transaction for reads (afaik?). So you could say that MULTI is effectively a write command, and as soon as you call that it switches to the master server.

SAVE and others yes indeed those should probably throw an exception. Otherwise it could lead to funky results. I think we should work with a whitelist approach to avoid surprises. Anything that's not been clearly thought through and marked as supported => exception.

from predis.

nrk avatar nrk commented on June 2, 2024

After giving it a bit of though, I think it should be doable with a couple of more classes and a few lines of changes to the existing code. Not sure if this is the best way, but it's a start at least.

A virtual connection that internally holds one master and multiple slave connections, plus the logic to pick a slave to read from and decide which commands represent read-only ops and which ones modify data, could be easily defined as a superset of Predis\Network\IConnectionCluster with just a few more convenience methods, for example:

interface Predis\Network\IConnectionMasterSlave
    extends Predis\Network\IConnectionCluster {

    public function getMaster();
    public function setMaster(IConnectionSingle $connection);
    public function getSlaves();
}

This would make the class implementing the above interface easily integrate with the current client implementation. Internally the connection class should check for the command ID of a given instance of Predis\ICommand to decide where it should be routed to (with the already mentioned notable exception of the SORT command with the STORE flag set).

Everything looks all nice and dandy, but the problem of extending the Predis\Network\IConnectionCluster interface is that it disallow MULTI/EXEC on that connection (at least using the Predis\MultiExecContext abstraction). Also, the switch to the master connection should be performed as soon as a WATCH command is sent (which usually happens before MULTI, but it doesn't mean that a MULTI command will be issued at all, see the MultiExecTransactionsWithCAS.php for example).

Another doubt is represented by how this can be integrated into Predis\Client to define the configuration of master/slave servers in a decent way. The only way I've come up with so far is something like the following:

// new Predis\MasterSlaveConfiguration class
$parameters = new Predis\MasterSlaveConfiguration(array(
    'master' => 'tcp://127.0.0.1:6379',
    'slaves' => array(
        'tcp://10.0.0.1:6379',
        'tcp://10.0.0.2:6379',
    ),
);

$redis = new Predis\Client($parameters);

This can be hooked easily into the existing connection initialization code. Hmmm...

from predis.

Seldaek avatar Seldaek commented on June 2, 2024

Doesn't sound too bad to me. It may be slightly complicated to implement but it would be pretty awesome from the user point of view.

from predis.

nicam avatar nicam commented on June 2, 2024

Did this get implemented yet?

from predis.

nrk avatar nrk commented on June 2, 2024

The master/slave feature has not been implemented yet but I had a couple of additional ideas since my last comment on this thread. I will probably start throwing some code together on a separate branch in a couple of weeks, right now I'm in the middle of implementing the support for redis-cluster.

from predis.

cj avatar cj commented on June 2, 2024

Any update on this?

from predis.

nrk avatar nrk commented on June 2, 2024

Not yet, in the end I couldn't produce anything that convinced me and I prefer to avoid pushing half-baked features. I'm probably going to revise my plans and release a stable v0.7.0 within a few weeks without support for redis-cluster, postponed to v0.8.0 since it's coming late compared to the initial plans. Supporting redis-cluster will require a few changes in Predis anyway, so I'll take the chance and try to design a decent master/slave replication handling in v0.8.0.

from predis.

snc avatar snc commented on June 2, 2024

You may have a look at the PECL/mysqlnd_ms extension (http://blog.ulf-wendel.de/) for some inspirations...

from predis.

nrk avatar nrk commented on June 2, 2024

Thanks for the pointer I'll have a look at it, but I think that the problem here is related to how Predis is currently designed more than anything else. We cannot do too much fancy stuff or runtime checks for performance reasons, so it's a bit more complicated to get it done decently. Anyway we'll see, I'd definitely like to add such a feature.

from predis.

nrk avatar nrk commented on June 2, 2024

Alright I think I've finally come up with a decent idea to implement this proposal on top of Predis v0.7.0 (which has been released yesterday). Right now it's just a gut feeling but I guess it could even make it into the next patch release (v0.7.1) since it shouldn't bring any breaking change into the existing code, but we need more investigation to be able to tell.

This is the basic idea:

  • Add a new client option named replication or something like that (if you have a better name for it...). If set, Predis will work in master/slave mode with one master and multiple slaves.
  • Make this new option work pretty much like the cluster one by accepting true, the FQN of a class or a callable object acting as a lazy initializer.
  • Implement a new interface, Predis\Network\IReplicationConnection (better name?), extending Predis\Network\IConnection (we are going the explicit route here).
  • Use the master connection parameter to specify which one must be considered the master. If none of the items in the parameters array has master set, we throw an exception (we need a master after all).
  • Switch to master as soon as we hit a command that performs a write operation. To decide if a command performs write operations or not we work on an exception basis: by default we consider every command as one that performs write operations.
  • Switch to master as soon as we hit WATCH or MULTI.
  • To support command pipelining we probably need a separate pipeline executor.. should we switch to master as soon as we start the pipeline? We can't predict if we are going to hit a command that performs write operations after all.

For the end user, the code should look like the the following:

$parameters = array(
    'tcp://10.10.10.1:6379/?master=1,
    'tcp://10.10.10.2:6379/,
    'tcp://10.10.10.3:6379/,
);

$client = new Predis\Client($parameters, array('replication' => true));

Comments?

from predis.

Seldaek avatar Seldaek commented on June 2, 2024

Sounds great to me. I do not really know what it means for cluster, but please keep cluster stuff in mind to avoid getting stuck with a replication model that is not compatible or creates an awkward API for cluster, because I guess in the long term cluster will be the main solution.

from predis.

nrk avatar nrk commented on June 2, 2024

I'd rather avoid to implement this one if I have to change the behaviour of existing and well estabilished features in Predis.

As far as I can tell right now, nothing will change for client-side clustering: in Predis\Client we will decide early if the client has to work in cluster-mode or replication-mode by checking if the replication option has been explicitly specified by the user, so we can create a specific connection instance accordingly.

EDIT: oh and by the way this won't affect in any way the future development for redis-cluster (that is, the one that will be in Redis 3.0) since we need a couple of design changes in the library to support it anyway.

from predis.

nrk avatar nrk commented on June 2, 2024

Alright, I pushed a couple of commits in a separate branch adding support for master / slave replication. Tests are still missing and pipelining is broken for now, but aside from that it seems to work fine already.

In order to be able to use the replication mode you need a master and at least one slave. To specify the connection to the master server you can just use the alias connection parameter, which instead is optional for slaves. Here is an example:

<?php
$parameters = array(
    'tcp://host1:6379?alias=master',
    'tcp://host2:6379?alias=slave1',
    'tcp://host3:6379?alias=slave2',
);

$client = new Predis\Client($parameters, array('replication' => true));

A few additional notes for my previous comment:

  • SORT is a read-only command except when the STORE modifier is present. We handle this case and switch to the master connection only on STORE.
  • EVAL and EVALSHA are considered as commands that perform write operations since it's not possible for us to know what a Lua script actually does.
  • It'd be probably better to disable commands such as MONITOR or INFO, I'm not sure if it makes sense to be able to call them when we don't know which server we are actually connected to.

I'll work a bit more on this in the next few days.

from predis.

Seldaek avatar Seldaek commented on June 2, 2024

Regarding EVAL, I think that's a smart default, but maybe a way to mark something as readonly would be good. Because that's typically something you use for performance reasons, and having it go to the master always defeats that purpose a bit.

from predis.

nrk avatar nrk commented on June 2, 2024

It makes perfectly sense, but right now I can't think of anything to make it possible to differentiate between an EVAL command that carries a script performing write operations and one that doesn't.

Well actually I do have an idea that could work, though it probably feels a bit awkward for the initialization part: expose a method that allows to mark a Lua script as a read-only operation using its SHA1 hash instead of the command ID (this would make it work for both EVAL and EVALSHA).

from predis.

nrk avatar nrk commented on June 2, 2024

I can't really think of anything other than what I implemented with this commit to mark Lua scripts for EVAL / EVALSHA as read-only operations:

<?php
$script = "return redis.call('info', ARGV[1])";

$options = array(
    'profile' => 'dev',
    'replication' => function() use($script) {
        $replication = new Predis\Network\PredisReplication();
        $replication->setScriptReadOnly($script);
        return $replication;
    },
);

$client = new Predis\Client(/* your server list */, $options);

$client->eval($script, 0, 'replication');
$client->evalsha(sha1($script), 0, 'replication');

Honestly it's not as bad as I initially thought, and since the replication option can accept any kind of callable object you can just use an external class that responds to __invoke() to configure the instance of Predis\Network\PredisReplication with a self-contained solution.

from predis.

nrk avatar nrk commented on June 2, 2024

It looks like support for master / slave replication will make it into v0.7.1 after all :-)

As you can see from the most basic example included in the repository the client configuration is pretty simple, but it's also flexible enough to allow more complex configurations.

I can finally close this issue, thanks everyone for the feedback!

from predis.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.