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.
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.
So I guess we could take one step further and remove Redis Transaction all the way.