Permalink
Browse files

use "redis-lock" to prevent url collisions

  • Loading branch information...
1 parent eee6743 commit a07ab67df071d04d79cd38d352c992a3cb7066a4 @TooTallNate TooTallNate committed Sep 5, 2012
Showing with 73 additions and 44 deletions.
  1. +72 −44 app.js
  2. +1 −0 package.json
View
@@ -28,6 +28,14 @@ redis = require('redis').createClient(
);
/**
+ * Redis lock.
+ * Uses "shorty-lock" as the key name.
+ * Default timeout of 10 seconds.
+ */
+
+var lock = require('redis-lock')(redis).bind(null, 'shorty-lock', 10000);
+
+/**
* Create app.
*/
@@ -123,35 +131,51 @@ app.get('/', function (req, res, next) {
app.post('/', validate, exists, function (req, res, next) {
var url = req.body.url
, parsed = req.body.parsed
+ , length
+ , short
+ , obj
+
+ lock(function (unlock) {
+ // get count of urls
+ redis.hlen('urls', onLenth);
+ function onLenth (err, len) {
+ if (err) return next(500);
+ length = len;
+ short = base60.toString(length ? length + 1 : 0);
+
+ // next save the short url with the original url to the "urls" hash
+ redis.hset('urls', short, url, onUrlsSet);
+ }
+ function onUrlsSet (err) {
+ if (err) return next(err);
- // get count of urls
- redis.hlen('urls', function (err, length) {
- if (err) return next(500);
-
- var short = base60.toString(length ? length + 1 : 0);
-
- redis.hset('urls', short, url, function (err) {
+ // next save the original url with the short url to the "urls-hash" hash
+ redis.hset('urls-hash', url, short, onUrlsHashSet);
+ }
+ function onUrlsHashSet (err) {
if (err) return next(err);
- redis.hset('urls-hash', url, short, function (err) {
- if (err) return next(err);
-
- var obj = {
- type: 'url created'
- , url: url
- , short: short
- , date: new Date
- };
-
- redis.lpush('transactions', JSON.stringify(obj), function (err) {
- if (err) return next(500);
-
- obj.parsed = parsed;
- io.of('/main').volatile.emit('total', length + 1);
- io.of('/stats').volatile.emit('url created', short, parsed, Date.now());
- res.send({ short: 'https://' + app.set('domain') + '/' + short });
- });
- });
- });
+
+ // finally create a "transaction" object for this action
+ obj = {
+ type: 'url created'
+ , url: url
+ , short: short
+ , date: new Date
+ };
+
+ // push the transaction object to the "transitions" list
+ redis.lpush('transactions', JSON.stringify(obj), onTransactions);
+ }
+ function onTransactions (err) {
+ if (err) return next(500);
+
+ obj.parsed = parsed;
+ io.of('/main').volatile.emit('total', length + 1);
+ io.of('/stats').volatile.emit('url created', short, parsed, Date.now());
+ res.send({ short: 'https://' + app.set('domain') + '/' + short });
+
+ process.nextTick(unlock);
+ }
});
});
@@ -228,25 +252,29 @@ app.get('/:short', function (req, res, next) {
if (err) return next(err);
if (!val) return res.render('404');
- redis.lpush('transactions', JSON.stringify({
- type: 'url visited'
- , url: val
- , short: req.params.short
- , date: Date.now()
- , ip: req.socket.remoteAddress
- , headers: req.headers
- }), function (err) {
- if (err) console.error(err);
- });
+ lock(function (unlock) {
+ redis.lpush('transactions', JSON.stringify({
+ type: 'url visited'
+ , url: val
+ , short: req.params.short
+ , date: Date.now()
+ , ip: req.socket.remoteAddress
+ , headers: req.headers
+ }), function (err) {
+ if (err) console.error(err);
+ });
+
+ io.of('/stats').volatile.emit(
+ 'url visited'
+ , req.params.short
+ , url.parse(val)
+ , Date.now()
+ );
- io.of('/stats').volatile.emit(
- 'url visited'
- , req.params.short
- , url.parse(val)
- , Date.now()
- );
+ res.redirect(val);
- res.redirect(val);
+ process.nextTick(unlock);
+ });
});
});
View
@@ -9,6 +9,7 @@
"express": "~2.5.10"
, "socket.io": "~0.9.6"
, "redis": "~0.7.2"
+ , "redis-lock": "~0.0.2"
, "jade": "~0.26.3"
, "jadevu": "~0.0.7"
, "stylus": "~0.28.2"

4 comments on commit a07ab67

@tj
Member
tj commented on a07ab67 Sep 5, 2012

is this something .multi() could fix? I dont get what the lock is locking

@rauchg
Contributor
rauchg commented on a07ab67 Sep 5, 2012

multi wouldn't fix it because we need to get a value and act on it. I think the only way to encode these operations atomically alternatively would be with a lua script.

@tj
Member
tj commented on a07ab67 Sep 5, 2012

ah, just couldn't tell from the diff what's going on

@TooTallNate
Contributor

Locking in the '/:short' route is probably overkill, but ya for the / route we do a "hlen" and then an "hset", so we need to make sure that set doesn't get touched in between the two.

Please sign in to comment.