From 6c5b0da20dbe39e423fc6313232342a864311ef6 Mon Sep 17 00:00:00 2001 From: Gene Ng Date: Tue, 21 Apr 2020 22:42:55 +0800 Subject: [PATCH] Fix bug that scripts may not be loaded correctly in pipeline --- lib/pipeline.ts | 9 +++++-- test/functional/pipeline.ts | 49 ++++++++++++++++++++++++------------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/lib/pipeline.ts b/lib/pipeline.ts index 8d94cd90..8882d674 100644 --- a/lib/pipeline.ts +++ b/lib/pipeline.ts @@ -253,6 +253,7 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) { // Check whether scripts exists const scripts = []; + const addedScriptHashes = {}; for (let i = 0; i < this._queue.length; ++i) { const item = this._queue[i]; if (this.isCluster && item.isCustomCommand) { @@ -267,10 +268,11 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) { continue; } const script = this._shaToScript[item.args[0]]; - if (!script) { + if (!script || addedScriptHashes[script.sha]) { continue; } scripts.push(script); + addedScriptHashes[script.sha] = true; } const _this = this; @@ -279,7 +281,10 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) { } return this.redis - .script("exists", Array.from(new Set(scripts.map(({ sha }) => sha)))) + .script( + "exists", + scripts.map(({ sha }) => sha) + ) .then(function (results) { const pending = []; for (let i = 0; i < results.length; ++i) { diff --git a/test/functional/pipeline.ts b/test/functional/pipeline.ts index 7a942631..375c009e 100644 --- a/test/functional/pipeline.ts +++ b/test/functional/pipeline.ts @@ -267,8 +267,8 @@ describe("pipeline", function () { it("should check and load uniq scripts only", function (done) { const redis = new Redis(); redis.defineCommand("test", { - numberOfKeys: 1, - lua: "return {unpack(KEYS),unpack(ARGV)}", + numberOfKeys: 2, + lua: "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}", }); redis.defineCommand("echo", { numberOfKeys: 1, @@ -276,20 +276,30 @@ describe("pipeline", function () { }); redis.once("ready", function () { - const expectedComands = [ - "script", - "script", - "script", - "evalsha", - "evalsha", - "evalsha", - "evalsha", + const expectedCommands = [ + ["script", "exists"], + ["script", "load", "return {KEYS[1],ARGV[1]}"], + ["script", "load", "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}"], + ["evalsha"], + ["evalsha"], + ["evalsha"], + ["evalsha"], + ["evalsha"], + ["evalsha"], + ]; + const expectedResults = [ + [null, ["a", "1"]], + [null, ["b", "2"]], + [null, ["k1", "k2", "v1", "v2"]], + [null, ["k3", "k4", "v3", "v4"]], + [null, ["c", "3"]], + [null, ["k5", "k6", "v5", "v6"]], ]; redis.monitor(function (err, monitor) { monitor.on("monitor", function (_, command) { - const name = expectedComands.shift(); - expect(name).to.eql(command[0]); - if (!expectedComands.length) { + const expectedCommand = expectedCommands.shift(); + expectedCommand.forEach((arg, i) => expect(arg).to.eql(command[i])); + if (!expectedCommands.length) { monitor.disconnect(); redis.disconnect(); done(); @@ -297,11 +307,16 @@ describe("pipeline", function () { }); const pipe = redis.pipeline(); pipe - .echo("f", "0") - .test("a", "1") + .echo("a", "1") .echo("b", "2") - .test("b", "3") - .exec(); + .test("k1", "k2", "v1", "v2") + .test("k3", "k4", "v3", "v4") + .echo("c", "3") + .test("k5", "k6", "v5", "v6") + .exec(function (err, results) { + expect(err).to.eql(null); + expect(results).to.eql(expectedResults); + }); }); }); });