Clear the lock only if it was created by the same process

[rohitm]

redis-lock/index.js

Line 44 in dcfa15f

if(lockTimeoutValue > Date.now()) {

I've been testing your lock coupled with an expressjs server and the lock doesn't work if you pass concurrent requests to the webserver. If you look at the correct implementation prescribed on the Redis website, you'll find that lock should be cleared only by the process or routine that acquired the lock in the first place.

I therefore landed up rewriting the lock / unlock logic to suit my need. I think your code will "better" guarantee mutual exclusion if you don't rely on timestamps for lock acquisition and clearance. Instead use a process ID or a randomly generated UUID.

// Set lock by randomUUID on line 15
// TODO : Generate randomUUID and save it in somewhere in the lock's context
client.set(lockName, randomUUID, 'PX', timeout, 'NX', function(err, result) { ...


// Clear lock by verifying the lock's creator on line 44
// TODO : Figure out who acquired the lock from redis 'GET lockName'
if(lockSetByUUID === randomUUID) {
	client.del(lockName, done);
...	

I hope this helps. Cheerio!

[rohitm]

Here is my implementation ; I didn't care for retries.

module.exports = ({redisCli, lockName, uniqueId }) => new Promise((resolve) => {
	redisCli.send_command('SET', [lockName, uniqueId, 'PX', '5000', 'NX'], async (err, res) => {
		if (err || res === null) {
			return resolve(() => { return; });
		}
		const lockSettor = await redisCli.getAsync(lockName);
		if (lockSettor !== uniqueId) {
			return resolve(() => { return; });
		}

		resolve(async function unLock() {
			await redisCli.del(lockName);
		});
	});		
});

// usage 
const release = await lock({ redisCli, lockName: 'emailQueueProcessorLock', uniqueId: req.id });
// Critical region begin
// .... Do something that consumes time
await release();

[rohitm]

You could further optimise the lock clearance logic by moving it outside of node and into Redis itself. Redis allows you to use lua for such purposes.

[rohitm]

Tagging @rakeshpai In case he didn't get notified.

[rakeshpai]

Hey @rohitm. Sorry, been busy.

Wouldn't this mean that that if the process crashes before it releases the lock, that's a deadlock situation since no one will now be able to acquire the lock?

[rohitm]

Hey @rohitm. Sorry, been busy.

Wouldn't this mean that that if the process crashes before it releases the lock, that's a deadlock situation since no one will now be able to acquire the lock?

Right, So in the deadlock situation, you've set the lock to expire in 5 seconds by default.

redis-lock/index.js

Line 15 in dcfa15f

client.set(lockName, lockTimeoutValue, 'PX', timeout, 'NX', function(err, result) {

After which other processes will be able to continue creating a lock. You should still be good. The recommendation to clear the lock based on the creator isn't novel or mine, its from redis. One can always alter the default 5 seconds to fit their need.