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. lib/distributed_mutex.rb 363170513 redis.synchronize do redis.unwatch redis.watch key current_expire_time = redis.get key if current_expire_time && now <= current_expire_time.to_i redis.unwatch got_lock = false else result = redis.multi do redis.set key, expire_time.to_s redis.expireat key, expire_time + 1 end got_lock = !result.nil? end This file has been truncated. show original How we may improve it Both lock and 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 SETEX. 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 & EXEC did not provide anything. This is because that if the lock is not acquired, it will retry. A Historical note I dig into the git commit history for more information. It seems like that when the Redis lock is introduced, we did not use the TTL provided by Redis. The TTL feature is introduced much later. So I guess we could take one step further and remove Redis Transaction all the way.

A more efficient Redis lock

sam (Sam Saffron) January 31, 2021, 11:22pm 2

We can probably junk the whole transaction and do a set with with the px and nx options

Topic		Replies	Views
Timeouts of Redis Lock Development	1	600	August 24, 2021
Found unused variable `expires` at lib/secure_session.rb, it seem like a mistake Development	2	615	November 11, 2019
Redis Error after update Self-hosting	9	2570	March 5, 2022
Huge increase in Redis use after changing hosts Self-hosting	8	1491	August 10, 2018
Is there a way to change the redis connection timeout? Self-hosting	14	13655	January 4, 2019