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

ioredis 2: custom filter predicates for "scaleReads" #250

Closed
ramonsnir opened this issue Feb 9, 2016 · 5 comments
Closed

ioredis 2: custom filter predicates for "scaleReads" #250

ramonsnir opened this issue Feb 9, 2016 · 5 comments

Comments

@ramonsnir
Copy link

Great job, btw, both on ioredis 1 and ioredis 2!

It would be really great if it would be possible to give the "scaleReads" option a custom function that filters which nodes are applicable to be read from, in case not all nodes are born equal (e.g. some are closer to the application than others, network-wise).

Thinking about it, even a bit more:

  1. Giving 2 functions: one for readonly commands and one for readwrite commands.
  2. Supporting not only a yes/no boolean, but a 0-9 priority (so ioredis would prefer those with 9 over those with 8, etc. etc., but never those with 0). This is for more complicated Redis Cluster topologies.

If you agree that this is at least an acceptable feature, I could open a PR for you into the 2.x branch.

P.S.: shouldn't there be a way to mark eval/evalsha as readonly - so reading lua scripts won't be redirected to a master?

@luin
Copy link
Collaborator

luin commented Feb 9, 2016

Sounds interesting. Could you give me an example about the custom function case?

P.S. It seems that redis always consider EVAL and EVALSHA as writes, so the slaves will not handle them (https://github.com/antirez/redis/blob/unstable/src/cluster.c#L5178-L5184).

@ramonsnir
Copy link
Author

The one I'm personally having at the moment is that my cluster spans across multiple network subnets, and the latency between them is relatively large. So my function would take the node's IP, check if it's in the "local subnet IP range", and return a lower priority if it isn't. This would lower significantly the read times, as otherwise reads would go to a random slave which may or may not be nearby (i.e. 50ms latency instead of 1ms latency).

P.S.: That is too bad 😞 Yet another regression from classic Redis.

@luin
Copy link
Collaborator

luin commented Feb 9, 2016

That makes sense. What about a function with two parameters slaves and command? slaves is an array of the slaves serving the slot. Return an index of the array to send the command to the specified slave, or return a false value to send to master.

new Redis.Cluster([/* nodes */], {
  scaleReads: function (slaves, command) {
    if (command.name === 'get') {
      return 0;
    }
  }
});

@ramonsnir
Copy link
Author

That could work, although I thought that you'd prefer to keep as much logic as possible inside the library, just letting the custom function filter out low-prioritized nodes.

Your suggestion 100% solves the problem.

@luin
Copy link
Collaborator

luin commented Feb 10, 2016

Closing this issue in favour of #252.

@luin luin closed this as completed Feb 10, 2016
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

No branches or pull requests

2 participants