Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

defined commands don't work the first time within multi #104

Closed
ehacke opened this issue Jul 17, 2015 · 3 comments
Closed

defined commands don't work the first time within multi #104

ehacke opened this issue Jul 17, 2015 · 3 comments

Comments

@ehacke
Copy link

ehacke commented Jul 17, 2015

Looks like defined commands cannot be run for the first time inside of a multi block.

it('should be able to use defined scripts for the first time within multi', function (done) {
    var redis = new Redis();

    redis.defineCommand('echoDynamicKeyNumber', {
      lua: 'return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}'
    });

    redis.multi({ pipeline: false });
    redis.set('foo', 'bar');
    redis.get('foo');
    redis.echoDynamicKeyNumber(2, 'k1', 'k2', 'a1', 'a2');
    redis.exec(function (err, results) {
      expect(results).to.eql([[null, 'OK'], [null, 'bar'], [null, ['k1', 'k2', 'a1', 'a2']]]);
      done();
    });
  });

Produces: NOSCRIPT No matching script. Please use EVAL.

Is this intended or necessary behaviour? Seems like there are tests for pipeline, but not multi with no pipeline.

If I load a lua file with defineCommand, and execute the command at least once before the multi block, it works.

@ehacke
Copy link
Author

ehacke commented Jul 17, 2015

Seems to work fine with pipeline enabled.

Work-around that works for me

I only disabled pipeline because I don't know ahead of time the commands I'll be sending in the multi block. So I can't chain them.

For my purposes, rather than disabling pipeline, I can just build an array of commands and submit them as a batch to the multi constructor.

redis.defineCommand('echoDynamicKeyNumber', {
      lua: 'return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}'
    });

var commands = [];
commands.push(['echoDynamicKeyNumber', 2, 'k1', 'k2', 'a1', 'a2']);
if (somethingExists) {
  commands.push(['conditionalCommand', 'arg1', 'arg2']);
}
...
redis.multi(commands).exec(function(){..});

@luin
Copy link
Collaborator

luin commented Jul 17, 2015

Hi, NOSCRIPT occurs because exec is invoking before eval. You can refer to my comment to another issue: #91 (comment). The first time you run the following code:

redis.multi({ pipeline: false });
redis.set('foo', 'bar');
redis.get('foo');
redis.echoDynamicKeyNumber(2, 'k1', 'k2', 'a1', 'a2');
redis.exec();

Would actually sending commands to Redis server as the following order:

MULTI
SET foo bar
GET foo
EVALSHA
EXEC
EVAL

Since pipelining in ioredis is very flexible, you can use pipelining this way:

var pipeline = redis.multi().echoDynamicKeyNumber(2, 'k1', 'k2', 'a1', 'a2');
if (condition) {
  pipeline.get('foo');
}
pipeline.exec();

@ehacke
Copy link
Author

ehacke commented Jul 19, 2015

@luin thanks! That'll work for me.

You can close this if you'd like.

@luin luin closed this as completed Jul 20, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants