I recently had to make some performance optimizations. These are my favorite kind of coding task as they usually require some sort of creativity and tradeoff. This time around I also learned more about Redis and practiced using AI to generate code.

During my refactor, I came across a need to upsert a (member, score) into a Redis sorted set only if the Redis sorted set exists. This upsert would be done hundreds of times a second for different keys, so it had to be fast.

The basic ZADD command offers a XX flag which adds a (member, score) only if the member exists in the sorted set. But what I was looking for was a flag to add a (member, score) only if the sorted set exists. Unfortunately, no flag exists for this use case. If no sorted set exists, Redis will create a new one with just that (member, score). I didn’t want that.

If I didn’t care about atomicity, I could simply do:

if Redis.exists('myset'):
Redis.zadd('myset', {member: score})
return True
else:
return False

But I’m not so lucky. I figured this would be a good opportunity to use AI, something I’m gradually trying to get in the habit of using. I told it to make the above operation atomic. Here’s what it did:

p = Redis.pipeline()
p.exists('myset')
p.zadd('myset', {member: score})
result = p.execute()
if result[0]:  # Check if key existed
return True
return False

Pretty cheeky. Less correct than the first version. This will return the same value to the caller but it will always perform the ZADD operation. I think the AI interpreted the word ‘atomic’ as a cue to use a pipeline. I told the AI this doesn’t work and to try a different approach.

Then, to my surprise, it actually offered a correct solution. It generated a LUA script which does the check-and-set, registered the script, and called it. While this was technically correct, I wasn’t a fan of LUA because it would block Redis. And since this script would be called a lot, it may create performance issues elsewhere in my system. I told AI to achieve the same result without LUA.

This time, it introduced me to the WATCH command which I’d never heard of before. WATCH is a cool way to achieve a check-and-set while guarding against race conditions and without the overhead of LUA. It uses optimistic locking instead. This seemed sensible for my use case at first. But when I experimented with it, it didn’t actually work as promised.

ZADD myset 0 foo
EXPIRE myset 10
WATCH myset
MULTI
ZADD myset 1 bar
... wait 10 seconds ...
EXEC
ZRANGE myset 0 -1
1) "bar"

Redis transactions docs say:

So what is WATCH about? It is a command that will make the EXEC conditional: we are asking Redis to perform the transaction only if none of the WATCHed keys were modified. This includes modifications made by the client, like write commands, and by Redis itself, like expiration or eviction. If keys were modified between when they were WATCHed and when the EXEC was received, the entire transaction will be aborted instead.

In Redis versions before 6.0.9, an expired key would not cause a transaction to be aborted.

I was testing on Redis 6.2.1 so I should have this feature. But my transaction was not being aborted even though the TTL expired in the middle of it.

I did more research and found Redis docs on keyspace notifications which says:

Expired (expired) events are generated when the Redis server deletes the key and not when the time to live theoretically reaches the value of zero.

This may or may not be related to how WATCH works. But my hunch is that when TTL reaches zero, the WATCHed key is still considered unmodified because the Redis garbage collector hasn’t fired the expired event or evicted the key yet. I didn’t dig into the code though.

So I had to come up with another solution. It was a good reminder that AI generated code should never be taken at face value.

To summarize my design constraints:

1. A ZADD should only succeed on an existing key.
2. Not every possible ZADD had to be made. If I wasn’t sure a key exists, I could skip the ZADD and that would be okay.
3. I didn’t want to use LUA for performance reasons.

Allowances:

1. My design didn’t need to work under memory full conditions.
2. All my keys already had a TTL.

Following ‘the simplest thing that works’ maxim led me to my final approach:

if Redis.ttl('myset') > 10:
Redis.zadd('myset', {member: score})
return True
else:
return False

This only performs the ZADD if the key has a TTL of at least 10 seconds. Since I assume I will always have spare memory, the probability of Redis evicting a key between the call to TTL and ZADD (i.e. before the TTL expires) is zero. I also don’t have any other Redis commands that block Redis for 10 seconds, so I can safely assume the ZADD will be called within 10 seconds of my TTL check. I ‘lose’ ZADDs for keys which are expiring in 10 seconds, but for my use case, that’s tolerable.

There are many things I like about this approach:

1. It’s simple and easy to understand.
2. It’s performant. In practice, I would be batching the above in chunks of 200-300 keys at a time and using pipeline.
3. Even though it’s not theoretically perfect, it gets the job done 99.99% of the time.

In the 0.01% rare case that I create a rogue myset, it will be easy to detect because it will not have an associated TTL. Since I expect all my keys to have TTLs, a SCAN can clean out the keys which don’t.