feat: added support for route randomly flag #894
Conversation
| // Enabling it alongside client side caching can result in higher memory usage due to | ||
| // individual cache store against each connection. | ||
| // Only works when SendToReplicas return true | ||
| RouteRandomly bool |
There was a problem hiding this comment.
I think we don't need the new RouteRandomly option. We can just always invoke ReplicaSelector to pick a replica when necessary.
There was a problem hiding this comment.
The idea I had in my mind was, user should be in control if they want to route traffic across all the nodes (including master). If this flag is turned off then distribution will happen as it was happening earlier.
There was a problem hiding this comment.
If you want to make it default, then we can go with that approach. But we'd still need some flag to specify if master node should be included in distribution or not.
There was a problem hiding this comment.
Users already have the control with ReplicaSelector. They can return an out of range index, then we can send the command to the primary.
| rIndex := c.opt.ReplicaSelector(uint16(i), g.nodes[1:]) | ||
| if rIndex >= 0 && rIndex < n { | ||
| rslots[i] = conns[g.nodes[1+rIndex].Addr].conn | ||
| var replicas nodes |
There was a problem hiding this comment.
| var replicas nodes | |
| replicas := make(nodes, n) |
| rslots[i] = conns[g.nodes[1+rIndex].Addr].conn | ||
| var replicas nodes | ||
| if c.opt.RouteRandomly { | ||
| for _, node := range g.nodes { |
There was a problem hiding this comment.
| for _, node := range g.nodes { | |
| for _, node := range g.nodes[1:] { // exclude master node |
There was a problem hiding this comment.
Shouldn't we include master node for reads as well? That way we can route the traffic evenly across all the nodes for a given group.
No description provided.