From daa9e9e103118013bc3cb656d359305d3b45dd03 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Fri, 12 Dec 2014 17:50:07 -0800 Subject: [PATCH 01/13] change redis mutex impl. add ttl to the locks --- configs/.env | 1 + lib/models/redis/mutex.js | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/configs/.env b/configs/.env index 35585dde5..319b52470 100644 --- a/configs/.env +++ b/configs/.env @@ -1,6 +1,7 @@ LOG_ERRORS=true TOKEN_EXPIRES="1 year" REDIS_KEY_EXPIRES="5 minutes" +REDIS_LOCK_EXPIRES=600 DOCK_PROJECT_LIMIT=100000000 DEPLOY_COMMAND="echo DEPLOY" COOKIE_SECRET="\$up3r,$3 Date: Fri, 12 Dec 2014 18:14:52 -0800 Subject: [PATCH 02/13] prevent unlocking from random client --- lib/models/redis/mutex.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 86beaf25d..590dc611e 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -1,11 +1,15 @@ 'use strict'; var redis = require('./index'); +var uuid = require('uuid'); module.exports = RedisMutex; function RedisMutex (key) { this.redis = redis; this.key = key; + // token is used to prevent one client + // of removing lock set by another + this.token = uuid(); } RedisMutex.prototype.lock = function (cb) { @@ -13,11 +17,14 @@ RedisMutex.prototype.lock = function (cb) { // Is a simple way to implement a locking system with Redis. // See http://redis.io/commands/set var expires = process.env.REDIS_LOCK_EXPIRES; - this.redis.set(this.key, 'lock', 'NX', 'EX', expires, function (err, success) { + this.redis.set(this.key, this.token, 'NX', 'EX', expires, function (err, success) { cb(err, success === 'OK'); }); }; RedisMutex.prototype.unlock = function (cb) { - this.redis.del(this.key, cb); + var command = 'if redis.call("get",KEYS[1]) == ARGV[1] then'; + command += ' return redis.call("del",KEYS[1])'; + command += ' else return 0 end'; + this.redis['eval'](command, 1, this.key, this.token, cb); }; From ccbceb249c3f8f2c7f06e1e03cb6e6b48947b423 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Fri, 12 Dec 2014 18:21:11 -0800 Subject: [PATCH 03/13] fix lint error --- lib/models/redis/mutex.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 590dc611e..62492dee3 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -26,5 +26,7 @@ RedisMutex.prototype.unlock = function (cb) { var command = 'if redis.call("get",KEYS[1]) == ARGV[1] then'; command += ' return redis.call("del",KEYS[1])'; command += ' else return 0 end'; - this.redis['eval'](command, 1, this.key, this.token, cb); + /*jshint -W061 */ + this.redis.eval(command, 1, this.key, this.token, cb); + /*jshint +W061 */ }; From f5caca89b3529b334f87fdbc8dbd6302a8a95257 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 10:48:42 -0800 Subject: [PATCH 04/13] experimenting with locks --- lib/models/redis/mutex.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 62492dee3..19a428c7b 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -7,8 +7,6 @@ module.exports = RedisMutex; function RedisMutex (key) { this.redis = redis; this.key = key; - // token is used to prevent one client - // of removing lock set by another this.token = uuid(); } @@ -17,7 +15,7 @@ RedisMutex.prototype.lock = function (cb) { // Is a simple way to implement a locking system with Redis. // See http://redis.io/commands/set var expires = process.env.REDIS_LOCK_EXPIRES; - this.redis.set(this.key, this.token, 'NX', 'EX', expires, function (err, success) { + this.redis.set(this.key, 'lock', 'NX', 'EX', expires, function (err, success) { cb(err, success === 'OK'); }); }; @@ -26,7 +24,5 @@ RedisMutex.prototype.unlock = function (cb) { var command = 'if redis.call("get",KEYS[1]) == ARGV[1] then'; command += ' return redis.call("del",KEYS[1])'; command += ' else return 0 end'; - /*jshint -W061 */ - this.redis.eval(command, 1, this.key, this.token, cb); - /*jshint +W061 */ + this.redis['eval'](command, 1, this.key, 'lock', cb); }; From f75022f3da211ac93c28f6bc4614460d303fc9a9 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 10:55:41 -0800 Subject: [PATCH 05/13] experimenting with locks --- lib/models/redis/mutex.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 19a428c7b..7bbaaa24b 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -24,5 +24,7 @@ RedisMutex.prototype.unlock = function (cb) { var command = 'if redis.call("get",KEYS[1]) == ARGV[1] then'; command += ' return redis.call("del",KEYS[1])'; command += ' else return 0 end'; + /*jshint -W061 */ this.redis['eval'](command, 1, this.key, 'lock', cb); + /*jshint +W061 */ }; From 1c6a32f3147d24a97ee0e6f66d211a52bfcf792b Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 11:01:41 -0800 Subject: [PATCH 06/13] experimenting with locks --- lib/models/redis/mutex.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 7bbaaa24b..919196adf 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -15,7 +15,7 @@ RedisMutex.prototype.lock = function (cb) { // Is a simple way to implement a locking system with Redis. // See http://redis.io/commands/set var expires = process.env.REDIS_LOCK_EXPIRES; - this.redis.set(this.key, 'lock', 'NX', 'EX', expires, function (err, success) { + this.redis.set(this.key, this.token, 'NX', 'EX', expires, function (err, success) { cb(err, success === 'OK'); }); }; @@ -25,6 +25,6 @@ RedisMutex.prototype.unlock = function (cb) { command += ' return redis.call("del",KEYS[1])'; command += ' else return 0 end'; /*jshint -W061 */ - this.redis['eval'](command, 1, this.key, 'lock', cb); + this.redis['eval'](command, 1, this.key, this.token, cb); /*jshint +W061 */ }; From c482535bd86ec14cea32d923b994e004358ef739 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 11:09:11 -0800 Subject: [PATCH 07/13] experimenting with locks --- lib/models/redis/mutex.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/models/redis/mutex.js b/lib/models/redis/mutex.js index 919196adf..1705b2128 100644 --- a/lib/models/redis/mutex.js +++ b/lib/models/redis/mutex.js @@ -1,13 +1,11 @@ 'use strict'; var redis = require('./index'); -var uuid = require('uuid'); module.exports = RedisMutex; function RedisMutex (key) { this.redis = redis; this.key = key; - this.token = uuid(); } RedisMutex.prototype.lock = function (cb) { @@ -15,16 +13,11 @@ RedisMutex.prototype.lock = function (cb) { // Is a simple way to implement a locking system with Redis. // See http://redis.io/commands/set var expires = process.env.REDIS_LOCK_EXPIRES; - this.redis.set(this.key, this.token, 'NX', 'EX', expires, function (err, success) { + this.redis.set(this.key, 'lock', 'NX', 'EX', expires, function (err, success) { cb(err, success === 'OK'); }); }; RedisMutex.prototype.unlock = function (cb) { - var command = 'if redis.call("get",KEYS[1]) == ARGV[1] then'; - command += ' return redis.call("del",KEYS[1])'; - command += ' else return 0 end'; - /*jshint -W061 */ - this.redis['eval'](command, 1, this.key, this.token, cb); - /*jshint +W061 */ -}; + this.redis.del(this.key, cb); +}; \ No newline at end of file From bed5f06279b394e42841f2d315de8ef01dcc540e Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 11:20:45 -0800 Subject: [PATCH 08/13] add tests for the redis mutex --- unit/redis-mutex.js | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 unit/redis-mutex.js diff --git a/unit/redis-mutex.js b/unit/redis-mutex.js new file mode 100644 index 000000000..9189de6f0 --- /dev/null +++ b/unit/redis-mutex.js @@ -0,0 +1,53 @@ +var Lab = require('lab'); +var describe = Lab.experiment; +var it = Lab.test; +var expect = Lab.expect; + + +var RedisMutex = require('models/redis/mutex'); + + +describe('RedisMutex', function () { + + + describe('lock', function () { + + it('should lock', function (done) { + var mutex = new RedisMutex('key-1'); + mutex.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(true); + done(); + }); + }); + + it('should fail to lock with the same key', function (done) { + var mutex = new RedisMutex('key-1'); + mutex.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(false); + done(); + }); + }); + + describe('unlock', function () { + + it('should be able to lock after unlock', function (done) { + var mutex = new RedisMutex('key-1'); + mutex.unlock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal('1'); + mutex.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(true); + done(); + }); + }); + }); + + }); + + }); + + +}); From 2c6afd88356efc7e64991d6e2a39503230b335d8 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 11:29:36 -0800 Subject: [PATCH 09/13] better tests for the redis mutex --- unit/redis-mutex.js | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/unit/redis-mutex.js b/unit/redis-mutex.js index 9189de6f0..c4e7219cf 100644 --- a/unit/redis-mutex.js +++ b/unit/redis-mutex.js @@ -2,13 +2,15 @@ var Lab = require('lab'); var describe = Lab.experiment; var it = Lab.test; var expect = Lab.expect; - +var before = Lab.before; +var after = Lab.after; +var createCount = require('callback-count'); var RedisMutex = require('models/redis/mutex'); describe('RedisMutex', function () { - + var ctx = {}; describe('lock', function () { @@ -47,6 +49,41 @@ describe('RedisMutex', function () { }); + + describe('ttl', function () { + before(function (done) { + ctx.originREDIS_LOCK_EXPIRES = process.env.REDIS_LOCK_EXPIRES; + done(); + }); + after(function (done) { + ctx.originREDIS_LOCK_EXPIRES = process.env.REDIS_LOCK_EXPIRES; + done(); + }); + + it('should release lock after expiration time', function (done) { + var count = createCount(2, done); + process.env.REDIS_LOCK_EXPIRES = 500; + var mutex1 = new RedisMutex('key-1'); + var mutex2 = new RedisMutex('key-1'); + setTimeout(function () { + mutex2.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(false); + count.next(); + }); + }, 500); + mutex1.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(false); + mutex2.lock(function (err, success) { + if (err) { return done(err); } + expect(success).to.equal(false); + count.next(); + }); + }); + }); + }); + }); From 79b98f46c5d725777b5c04d7e325b57bfb54153f Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 12:11:13 -0800 Subject: [PATCH 10/13] fix reset error --- unit/redis-mutex.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit/redis-mutex.js b/unit/redis-mutex.js index c4e7219cf..3faadc7e6 100644 --- a/unit/redis-mutex.js +++ b/unit/redis-mutex.js @@ -56,7 +56,7 @@ describe('RedisMutex', function () { done(); }); after(function (done) { - ctx.originREDIS_LOCK_EXPIRES = process.env.REDIS_LOCK_EXPIRES; + process.env.REDIS_LOCK_EXPIRES = ctx.originREDIS_LOCK_EXPIRES; done(); }); From 597ec0f8f7a480b3cd9ba85348ddf2c6efa398b1 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Mon, 15 Dec 2014 17:24:33 -0800 Subject: [PATCH 11/13] use eson, use millis for ttl, fix test, decrease ttl in the test --- configs/.env | 2 +- lib/models/redis/mutex.js | 2 +- unit/redis-mutex.js | 17 ++++++++++------- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/configs/.env b/configs/.env index 319b52470..fe29ce412 100644 --- a/configs/.env +++ b/configs/.env @@ -1,7 +1,7 @@ LOG_ERRORS=true TOKEN_EXPIRES="1 year" REDIS_KEY_EXPIRES="5 minutes" -REDIS_LOCK_EXPIRES=600 +REDIS_LOCK_EXPIRES="6 minutes" DOCK_PROJECT_LIMIT=100000000 DEPLOY_COMMAND="echo DEPLOY" COOKIE_SECRET="\$up3r,$3 Date: Tue, 16 Dec 2014 12:44:12 -0800 Subject: [PATCH 12/13] decrease lock expire for the test --- unit/redis-mutex.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unit/redis-mutex.js b/unit/redis-mutex.js index 7bde5e587..99fb37f56 100644 --- a/unit/redis-mutex.js +++ b/unit/redis-mutex.js @@ -65,7 +65,7 @@ describe('RedisMutex', function () { it('should release lock after expiration time', function (done) { var count = createCount(2, done); - process.env.REDIS_LOCK_EXPIRES = 200; + process.env.REDIS_LOCK_EXPIRES = 50; var mutex1 = new RedisMutex('new-key-1'); var mutex2 = new RedisMutex('new-key-1'); setTimeout(function () { @@ -74,7 +74,7 @@ describe('RedisMutex', function () { expect(success).to.equal(true); count.next(); }); - }, 200); + }, 50); mutex1.lock(function (err, success) { if (err) { return done(err); } expect(success).to.equal(true); From dfe98311e3db25de49707a707207cd0abc352436 Mon Sep 17 00:00:00 2001 From: Anton Podviaznikov Date: Tue, 16 Dec 2014 12:52:08 -0800 Subject: [PATCH 13/13] bump set tiemout for ttl verification --- unit/redis-mutex.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit/redis-mutex.js b/unit/redis-mutex.js index 99fb37f56..8b9ffe173 100644 --- a/unit/redis-mutex.js +++ b/unit/redis-mutex.js @@ -74,7 +74,7 @@ describe('RedisMutex', function () { expect(success).to.equal(true); count.next(); }); - }, 50); + }, 100); mutex1.lock(function (err, success) { if (err) { return done(err); } expect(success).to.equal(true);