From 8d2426f86fa52bef234b00c24df64f663f851b86 Mon Sep 17 00:00:00 2001 From: Brad Vogel <3333775+Leonardcd88@users.noreply.github.com> Date: Sat, 15 Oct 2016 11:58:17 -0700 Subject: [PATCH 1/3] Add new 'removeOnComplete' option to atomically remove a job when it is completed (useful for high-volume queues). Fixes https://github.com/OptimalBits/bull/issues/354 --- README.md | 2 ++ lib/job.js | 2 +- lib/scripts.js | 27 +++++++++++++++------------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4907a2c..52a99d9 100644 --- a/README.md +++ b/README.md @@ -433,6 +433,8 @@ __Arguments__ If you use this option, it is up to you to ensure the jobId is unique. If you attempt to add a job with an id that already exists, it will not be added. + removeOnComplete {Boolean} A boolean which, if true, removes the job when it successfully + completes. Default behavior is to keep the job in the completed queue. } returns {Promise} A promise that resolves when the job has been succesfully added to the queue (or rejects if some error occured). On success, the promise diff --git a/lib/job.js b/lib/job.js index 1a2f4cf..af9bbb7 100644 --- a/lib/job.js +++ b/lib/job.js @@ -162,7 +162,7 @@ Job.prototype.delayIfNeeded = function(){ Job.prototype.moveToCompleted = function(returnValue, token){ this.returnvalue = returnValue || 0; - return scripts.moveToCompleted(this, token || 0); + return scripts.moveToCompleted(this, token || 0, this.opts.removeOnComplete); }; Job.prototype.moveToFailed = function(err){ diff --git a/lib/scripts.js b/lib/scripts.js index f23de70..8507af6 100644 --- a/lib/scripts.js +++ b/lib/scripts.js @@ -134,7 +134,7 @@ var scripts = { return execScript.apply(scripts, args); }, - moveToCompleted: function(job, token){ + moveToCompleted: function(job, token, removeOnComplete){ var keys = _.map([ 'active', 'completed', @@ -143,20 +143,23 @@ var scripts = { return job.queue.toKey(name); } ); - keys.push(job.lockKey()); - // - // INVESTIGATE: Should'nt we check if we have the lock before trying to move the - // job? - // + var deleteJob = 'redis.call("DEL", KEYS[3])'; + var moveJob = [ + 'redis.call("SADD", KEYS[2], ARGV[1])', + 'redis.call("HSET", KEYS[3], "returnvalue", ARGV[2])', + ].join('\n'); + var script = [ - 'if redis.call("EXISTS", KEYS[3]) == 1 then', - ' redis.call("SADD", KEYS[2], ARGV[1])', - ' redis.call("HSET", KEYS[3], "returnvalue", ARGV[2])', - ' redis.call("LREM", KEYS[1], 0, ARGV[1])', - ' if redis.call("get", KEYS[4]) == ARGV[3] then', // Remove the lock - ' return redis.call("del", KEYS[4])', + 'if redis.call("EXISTS", KEYS[3]) == 1 then', // Make sure job exists + ' if redis.call("GET", KEYS[4]) == ARGV[3] then', // Makes sure we own the lock + ' redis.call("LREM", KEYS[1], 0, ARGV[1])', + removeOnComplete ? deleteJob : moveJob, + ' redis.call("DEL", KEYS[4])', + ' return 0', + ' else', + ' return -1', ' end', 'else', ' return -1', From 6a96362565b0b71b68ef04061d4d0d2fa831f539 Mon Sep 17 00:00:00 2001 From: Brad Vogel <3333775+Leonardcd88@users.noreply.github.com> Date: Sat, 15 Oct 2016 15:20:48 -0700 Subject: [PATCH 2/3] Micro-optimization: Since we can assume there's only one job id in the active list, and that jobs being completed are likely near the end of the list, then we can stop iterating the list after finding the first element starting at the end. --- lib/scripts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/scripts.js b/lib/scripts.js index 8507af6..5ca3623 100644 --- a/lib/scripts.js +++ b/lib/scripts.js @@ -154,7 +154,7 @@ var scripts = { var script = [ 'if redis.call("EXISTS", KEYS[3]) == 1 then', // Make sure job exists ' if redis.call("GET", KEYS[4]) == ARGV[3] then', // Makes sure we own the lock - ' redis.call("LREM", KEYS[1], 0, ARGV[1])', + ' redis.call("LREM", KEYS[1], -1, ARGV[1])', removeOnComplete ? deleteJob : moveJob, ' redis.call("DEL", KEYS[4])', ' return 0', From 44bcd362513afc92731055c3d5b39dda0695d60f Mon Sep 17 00:00:00 2001 From: Brad Vogel <3333775+Leonardcd88@users.noreply.github.com> Date: Sat, 15 Oct 2016 15:23:32 -0700 Subject: [PATCH 3/3] Give script different name so it doesn't get cached with considering the removeOnComplete parameter. --- lib/scripts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/scripts.js b/lib/scripts.js index 5ca3623..4ff1057 100644 --- a/lib/scripts.js +++ b/lib/scripts.js @@ -168,7 +168,7 @@ var scripts = { var args = [ job.queue.client, - 'moveToCompletedSet', + 'moveToCompletedSet' + (removeOnComplete ? 'RemoveOnComplete' : ''), script, keys.length, keys[0],