Discourse use Redis lock widely as a way of synchronization. Discourse’s implementation of Redis lock(as show below) can be improved to use less Redis command, reducing round-trip time and cost for Redis to execute them.
How we may improve it
unlock uses Redis Transaction. the
lock process could be simplified into the code below.
WATCH key GET key # determine whether the key has expired MULTI SET key EXPIRE key, expire_time + 1
The reason to use Redis Transaction (I assumed) seems to be that it checks whether the key is expired before actually setting the key.
But I think we could just use the
SETEX command provided be Redis, which sets a key with a expiration time. In fact, the Redis SETEX Documentation use it as a example to replace
SET & EXPIRE with
Here is the arguments for why we should replace it:
- Setting the value to expiration time and checks it before setting the key is unnecessary. As the TTL mechanism is enough to ensure that key expires correctly.
- Even if we decide to use expiration time as value, we do not need any transaction. Since provding atomicity between the
GET(line 2 in code above) and
MULTI & EXECdid not provide anything. This is because that if the lock is not acquired, it will retry.
A Historical note
So I guess we could take one step further and remove Redis Transaction all the way.