Conversation
7ab8149 to
0dfc7f5
Compare
0a25c59 to
153589f
Compare
```
$options = [
'cluster' => 'redis',
// Enables reading from Redis Cluster replica nodes by mapping masters and replicas.
// and sending READONLY to any cluster node upon connection
// works only when using native Redis cluster mode (`'cluster' => 'redis'`)
'readonly' => true,
];
```
153589f to
13a6171
Compare
8b88d88 to
ca4244b
Compare
|
Hello @tillkruss and @vladvildanov. I've just slightly improved PR by supporting a few modes for scaling read operations in redis-cluster. In this version, read operations can be scaled between master and replica nodes in a |
… a few modes:
* `replicas` - send read operations to replicas nodes only
* `random` - send read operations to master and replicas nodes
```
$options = [
'cluster' => 'redis',
// Enables reading from Redis Cluster replica nodes by mapping masters and replicas.
// and sending READONLY to any cluster node upon connection
// works only when using native Redis cluster mode (`'cluster' => 'redis'`)
// Supports two modes:
// `replicas` - send read operations to replicas nodes only
// `random` - send read operations to master and replicas nodes
'scaleReadOperations' => 'replicas',
];
```
Moved a logic related to detection of an operation type (readonly or not) into a read connection selector
ca4244b to
601d3b5
Compare
|
Hello @tillkruss. Could you look into my changes and share your opinion about the feature? |
|
Looks like not breaking changes, which is great. I'd like @vladvildanov to review this and give his thumbs up. |
Hello @vladvildanov. Have you had a chance to look into the suggested changes? |
|
@disc Sorry for late response guys, I'm on it |
|
I see some major issues with this PR that will only worsen an existing problems in a client:
Adding new connection manager (e.g ReadConnectionSelector class) on top of existing RedisCluster, just to manage replica connections is a workaround that ignores the general issue that master/slave replication isn't supported. Load balancing is one of the features that comes up with replication, but the problem should be scoped accordingly - "we need to add a support for master/slave replication in Cluster API". And the access to master/replica nodes should be managed by Cluster object. https://github.com/predis/predis/blob/v2.x/src/Connection/Cluster/RedisCluster.php#L287
I disagree that we need an external class that should distinguish read and write commands. The information about the command mode should be exposed by command object itself, so the https://github.com/predis/predis/blob/v2.x/src/Replication/ReplicationStrategy.php#L213 Apart of it we have to specify separately if command is supported by cluster: https://github.com/predis/predis/blob/v2.x/src/Cluster/ClusterStrategy.php#L41 If we will add another detector, each new command that will be added needs to be appended to each of this detectors in 3 different places. So my point that we shouldn't make it even worse.
As I mentioned above, a separate class that will manage connections seems like workaround, connections are already managed by RedisCluster and new object should not share this responsibilities, also it will be aligned with Replication logic. |
Here's a proposal of scaling read operations in Redis Cluster by sending them to cluster's master or replicas nodes with a
READONLYcommand as init command for any connection.The option
scaleReadOperationssupports two modes:replicas- send read operations to replicas nodes onlyrandom- send read operations to master and replicas nodesHow to use:
Expected commands in
monitor(in case ofreplicasmode)READONLYto any random node (entry point from parameters)CLUSTER SLOTSto a selected connection aboveREADONLYto a random replica node (if it hasn't been sent before)GET my-keyto a selected replica connection above