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

Pipeline in cluster mode #416

Closed
ruofeilyu opened this issue Dec 22, 2016 · 6 comments
Closed

Pipeline in cluster mode #416

ruofeilyu opened this issue Dec 22, 2016 · 6 comments

Comments

@ruofeilyu
Copy link

ruofeilyu commented Dec 22, 2016

I have a task that need to chain 5000 commands in pipeline and I have to use the pipeline because the large number of commands, it would perform way better than sending command one by one. So the solution is to form different pipeline in client side based on the key. So i need to get the slot-node table in client.
Question 1
Where can i get the cached slot-node table variable in ioredis?
Question 2
Does ioredis only check the first command in pipeline and decide which server to go?

Much thanks. A suggestion for ioredis is to have more information about pipeling in cluster mode because a serious application using redis must also use cluster. Thanks.

@luin
Copy link
Collaborator

luin commented Jan 26, 2017

Sorry for the late response. I do not recommend to use pipeline in this case since the slot-node table changes and accessing slot-node on the application side breaks the slot redirection (ask, moved or node failovers).

For the question 2: ioredis checks the keys of all the commands to ensure that all these commands are able to sent to the same node.

For your case, just send the 5000 commands without pipeline should not cause any performance problems because Node.js by default follows pipeline mode (that sends the next command without waiting for the receiving of the previous command) when sending commands.

@luin luin closed this as completed Jan 26, 2017
@AVVS
Copy link
Collaborator

AVVS commented Jan 26, 2017

I've actually had to hack my way through this not so long ago. In my experience often cluster is used with pre-hashed keys, and I would want the pipeline to be formed for my requests to do some batching. I know that the requests would go to the same machine, hence for that case it would be reasonable to enable pipelining. @luin what do you think?

Example code with batching:

/**
 * Queues more requests to pipeline
 */
function addToPipeline(key) {
  // fetchData is a lua script
  this.pipeline.fetchData(1, key, this.omitFields);
}

module.exports.batch = function fetchDataBatch(keys, omitFields = []) {
  const timer = perf('fetchData:batch');
  const redis = this.redis;

  // this must include {}
  const prefix = redis.options.keyPrefix;
  const slot = calcSlot(prefix);
  // they will all refer to the same slot, because we prefix with {}
  // this has possibility of throwing, but not likely to since previous operations
  // would've been rejected already, in a promise this will result in a rejection
  const nodeKeys = redis.slots[slot];
  const masters = redis.connectionPool.nodes.master;
  const masterNode = nodeKeys.reduce((node, key) => node || masters[key], null);

  // uses internal implementation details
  if (is.fn(masterNode.fetchData) !== true) {
    masterNode.options.keyPrefix = prefix;
    masterNode.defineCommand('fetchData', { lua: fetchDataScript });
    masterNode.options.keyPrefix = null;
  }

  const pipeline = masterNode.pipeline();
  keys.forEach(addToPipeline, { pipeline, omitFields });

  return pipeline
    .exec()
    .tap(timer('pipeline'))
    .map(deserializePipeline)
    .finally(timer);
};

@luin
Copy link
Collaborator

luin commented Jan 26, 2017

@AVVS Why not just call Cluster#pipeline() directly instead of Redis#pipeline() given all your keys belong to the same slot since your prefix them with the common {} flag.

@AVVS
Copy link
Collaborator

AVVS commented Jan 26, 2017 via email

@luin
Copy link
Collaborator

luin commented Jan 26, 2017

@AVVS Aha! Then your code looks good to me. Why don't you just update your lua script to support multiple keys in one command?

@ruofeilyu
Copy link
Author

@luin
Because Node.js by default follows pipeline mode (that sends the next command without waiting for the receiving of the previous command) when sending commands.

Regarding this sentence, I think the advantage of pipeline is to save network bandwidth, hence reducing the time this task need. The node.js server performance is surely same in both mode(pipeline and send one by one) since it is asynchronous.

http://bbs.redis.cn/forum.php?mod=viewthread&tid=826
In the link above, they(youku) do the same thing(forming different pipeline to different node) and then merge together(they also check the slot-node table, I think that ask, moved is a rare thing, performance should be the first), otherwise cluster for caching is not an ideal option for sizeable application. I would hack it and pull requests later.

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

3 participants