thedeveloper/warlock

Battle-hardened distributed locking using redis

warlock's Issues

Retry support

From the docs, it looks like lock() is really more of an asynchronous tryLock() in that it fails immediately rather than returning when it finally acquires the lock. It would be nice if there was a built in retry mechanism, similar to what redis-lock has. Clients could optionally specify a retry delay between attempts to acquire the lock, and a maximum number of attempts to make before giving up.

Force unlock using either Redis or the API

I have waited like 5 minutes, and the the TTL for a key was only 10 seconds at most, I have also restarted Redis for good measure. But still for a certain key, I get:

'TypeError: unlock is not a function'

I saw this issue:
#15

There really should be a documented way to force unlock a key, either using Redis directly or with the Warlock API. There are so many uses cases for that. I recommend this:

standard API:

warlock.lock(key, ttl, callback);
unlock(callback);

extended API:

warlock.lock(key, options, callback);
warlock.unlock(options, callback);

options should give you the ability to do:

force:true

also, I am working on this project

https://github.com/ORESoftware/live-mutex

it would be really awesome if you could try it and critique it. Your project is much faster. But mine is good for developers who need similar locking mechanism but cannot installed Redis. Any feedback would be appreciated, even if it's criticism, thanks!

digest mismatch when unlocking

I am unlocking via unlock() however the unlock is not happening and I am seeing "digest mismatch" in the errors. any ideas?

Replace node-redis-scripty with node-redis-script

node-redis-scripty is deprecated in favor of node-redis-script.
@TheDeveloper Could this be updated in order to fix the security vulnerability reported in extend https://www.whitesourcesoftware.com/vulnerability-database/CVE-2018-16492 which is a dependency of node-redis-scripty?

Use random value instead of timestamp

Using a timestamp can lead to clients erroneously thinking they own a lock. Two possible examples are clients with bad clocks and checking for ownership of a lock <1ms after it was acquired.

The redis guide to distributed locks, which you link to in the documentation, has more info on the subject:

What this random string should be? I assume it’s 20 bytes from /dev/urandom, but you can find cheaper ways to make it unique enough for your tasks. For example a safe pick is to seed RC4 with /dev/urandom, and generate a pseudo random stream from that. A simpler solution is to use a combination of unix time with microseconds resolution, concatenating it with a client ID, it is not as safe, but probably up to the task in most environments.

But since everything's already async, I don't think it's a bid deal to just use crypto.pseudoRandomBytes.

Question: is it safe to reuse a previously used Redis client for Warlock?

I have a clustered Redis setup and besides using it as a data store, I also use it as a distributed socket.io adapter. Is it safe to reuse the same Redis client for warlock or should I create a separate one?

Below is my current Redis config:

const Redis = require('ioredis')
const redisAdapter = require('socket.io-redis')
const Warlock = require('node-redis-warlock')

const startupNodes = [{
    port: process.env.REDIS_PORT,
    host: process.env.REDIS_HOST
}]

const redis = new Redis.Cluster(startupNodes)

// pubClient can be the same client, but the subClient needs to be a new instance
const ioAdapter = redisAdapter({
    pubClient: redis,
    subClient: new Redis.Cluster(startupNodes)
})

// is this line safe?
const warlock = new Warlock(redis)

or should the last line be:
const warlock = new Warlock(new Redis.Cluster(startupNodes))

Thank you so much in advance! :)

-ERR wrong number of arguments for 'set' command

I keep on getting the error: "ERR wrong number of arguments for 'set' command."

Any ideas? I have attached the debug print out.

/usr/local/bin/node app.js
Queueing set for next server connection.
Queueing set for next server connection.
listening on *:3000
Stream connected 127.0.0.1:6379 id 1
Using default parser module: javascript
checking server ready state...
send 127.0.0.1:6379 id 1: *1
$4
info

send_command buffered_writes: 0 should_buffer: true
net read 127.0.0.1:6379 id 1: $1447

Server

redis_version:2.6.9
redis_git_sha1:00000000
redis_git_dirty:0
redis_mode:standalone
os:Darwin 14.3.0 x86_64
arch_bits:64
multiplexing_api:select
gcc_version:4.0.1
process_id:550
run_id:14b5a189f12ca0aad42ac89170b78b19d10b7516
tcp_port:6379
uptime_in_seconds:29
uptime_in_days:0
lru_clock:362954

Clients

connected_clients:1
client_longest_output_list:0
client_biggest_input_buf:0
blocked_clients:0

Memory

used_memory:699680
used_memory_human:683.28K
used_memory_rss:1740800
used_memory_peak:648384
used_memory_peak_human:633.19K
used_memory_lua:31744
mem_fragmentation_ratio:2.49
mem_allocator:libc

Persistence

loading:0
rdb_changes_since_last_save:0
rdb_bgsave_in_progress:0
rdb_last_save_time:1429692877
rdb_last_bgsave_status:ok
rdb_last_bgsave_time_sec:-1
rdb_current_bgsave_time_sec:-1
aof_enabled:0
aof_rewrite_in_progress:0
aof_rewrite_scheduled:0
aof_last_rewrite_time_sec:-1
aof_current_rewrite_time_sec:-1
aof_last_bgrewrite_status:ok

Stats

total_connections_received:1
total_commands_processed:0
instantaneous_ops_per_sec:0
rejected_connections:0
expired_keys:0
evicted_keys:0
keyspace_hits:0
keyspace_misses:0
pubsub_channels:0
pubsub_patterns:0
latest_fork_usec:0

Replication

role:master
connected_slaves:0

CPU

used_cpu_sys:0.04
used_cpu_user:0.03
used_cpu_sys_children:0.00
used_cpu_user_children:0.00

Keyspace

db0:keys=1,expires=0

Redis server ready.
Sending offline command: set
send 127.0.0.1:6379 id 1: *6
$3
set
$14
test-lock:lock
$24
Rceo0OjNEeSel88RZUsQ4w==
$2
PX
$5
10000
$2
NX

send_command buffered_writes: 0 should_buffer: false
Sending offline command: set
send 127.0.0.1:6379 id 1: *6
$3
set
$13
opt-lock:lock
$24
Rcf28OjNEeSel88RZUsQ4w==
$2
PX
$5
10000
$2
NX

send_command buffered_writes: 0 should_buffer: false
net read 127.0.0.1:6379 id 1: -ERR wrong number of arguments for 'set' command
-ERR wrong number of arguments for 'set' command

"SCRIPT is not implemented in fakeredis. Let me know if you need it."

I'd like to use warlock with fake_redis for a locking pattern, and I'm getting the error above. Is SCRIPT a vital part of warlock?

See https://github.com/hdachev/fakeredis/issues/15

Cannot call method 'run' of undefined

Can you help with this?
TypeError: Cannot call method 'run' of undefined
at /vagrant/node_modules/node-redis-warlock/lib/warlock.js:28:12

Seems like the scripty/lua stuff is failing...

Warlock fails when many concurrent requests happen

I am writing a library called live-mutex, it is much slower than using your library + Redis, and has a different use case, but apparently is more robust. Surely one of the biggest requirements of a locking library is to handle many concurrent requests to acquire a lock.

I have used this library in the past and it seemed to work, but since I had a test written for my library I decided to compare performance with other similar libraries.

Warlock should fail here, as is. If you change each to eachSeries it should succeed again.

You will need to start redis on default host/port and then run this:

https://github.com/ORESoftware/live-mutex/blob/master/test/speed/redis-warlock.js

Am I using the library incorrectly, or does Warlock fail upon many concurrent requests? Maybe there is a way to tune Warlock so it works in this case?

remove lock on exit

Hi,

How can users of this library remove the lock upon exit? In some cases, I am running a script and I kill the script before it completes and my guess is that Warlock does not release the Redis lock. How can we be guaranteed to be able to put a lock on a given Redis key?

Cannot swap the redis client

it seems Warlock( redisClient ) is using the very first redisClient passed to... and I can not create a new Warlock object with a new client in the same node.js process :)

Warlock( client1 )
client1.end()
Warlock( client2 )
// unlock is not working....

uuid module does not have version 1.4.1

Not sure if there ever was a v1.4.1, but now there isn't. You may wish to upgrade to the latest (https://github.com/defunctzombie/node-uuid/releases)

Allow wait to be a function

If wait can be a function, then you can easily implement things like exponential backoff. See https://github.com/gabegorelick/yarnl/#custom-backoff-strategies for example.

Force unlock

I would like to be able to force an unlock even if I don't know the id, is this possible?