A more efficient Redis lock

Development

AI-generated summary

Improving Redis Lock Efficiency in Discourse

The current implementation of Redis lock in Discourse can be optimized to reduce Redis command usage, round-trip time, and execution cost. The existing code uses Redis Transactions for lock and unlock operations, which can be simplified using the SETEX command.

concerto suggests that the current implementation is unnecessary, as the TTL mechanism ensures key expiration correctness. Using SETEX would eliminate the need for transactions, as the GET and MULTI & EXEC operations do not provide additional benefits.

sam agrees and references the Redis documentation on distributed locks, suggesting that the entire transaction can be replaced with a SET command using the px and nx options. This approach would further simplify the Redis lock implementation.

1

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

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:

  1. 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.
  2. 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.

2

Per Develop with Redis | Docs

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