Featured image of post Edit locks on Azure Managed Redis, and the slot error that broke them in production

Edit locks on Azure Managed Redis, and the slot error that broke them in production

Two people opened the same approval request. Both clicked approve. Both succeeded.

That is the whole bug report. The transport management system I work on has a screen called My approvals, where office users pick up requests that wait on them. Nothing stopped two users from picking the same one. The database was fine, the second write simply won, and nobody noticed until a customer asked why a request had two decisions on it.

I wanted a lock that any entity could use, without touching the entity tables. No LockedBy column, no cleanup job, no SQL migration for every new type. Redis was already in the estate, so the lock became a Redis lease.

The lock model

A lock is one JSON record under a key with a TTL.

1
2
key:   lock:{tenant}:{entityType}:{entityUid}
value: { entityType, entityUid, holderOid, holderName, acquiredAtUtc, expiresAtUtc }

The lease is five minutes. A client with the form open renews every two minutes. The holder is the Entra object id of the user, so the same person in two tabs counts as one holder. Expiry is the Redis TTL and nothing else; when a tab is closed the lock simply runs out. I would have liked a pagehide beacon to release early, but a beacon cannot carry a bearer token and the API needs one. Five minutes is an acceptable price for not building that.

Every write is a Lua script, so the lock key and its index never disagree. Claim is SET NX PX plus an index entry. Renew and release compare the stored holder before touching anything.

1
2
3
4
5
-- claim: KEYS[1] lock key, KEYS[2] index. ARGV[1] json, ARGV[2] lease ms, ARGV[3] expiry score
if not redis.call('SET', KEYS[1], ARGV[1], 'NX', 'PX', ARGV[2]) then return 0 end
redis.call('ZADD', KEYS[2], ARGV[3], KEYS[1])
redis.call('PEXPIRE', KEYS[2], ARGV[2])
return 1
1
2
3
4
5
6
-- release: only the holder may drop the lock
local v = redis.call('GET', KEYS[1])
if not v then return 0 end
if cjson.decode(v)['holderOid'] ~= ARGV[1] then return 0 end
redis.call('ZREM', KEYS[2], KEYS[1])
return redis.call('DEL', KEYS[1])

The scripts read the JSON with cjson only to compare the holder. The value written back is built by .NET, so the shape stays exactly what the clients deserialize. I learned the hard way, years ago, that letting Lua rewrite JSON produces subtle differences in number formatting.

Entities opt in with an attribute.

1
2
[Lockable]
public class ApprovalRequest : BaseEntity { ... }

The lock controller refuses claims for a type without the attribute, so a frontend that names an unmarked entity fails loudly in development instead of quietly holding a lock nobody enforces.

Enforcement sits at the API edge, on purpose

The generic PUT and DELETE answer 409 when another user holds a live lock, and the body carries the lock so the client can show who has it. The approval mutators do the same, and a successful approve or reject releases the lock.

A free entity is writable without a lock. This matters more than it sounds. Automation, the Functions worker and the driver app never claim locks and are never blocked by one. A driver reporting a delivery from the road must not lose that report because an office user has the order open on a second monitor. The lock protects a person who is editing; it is not a prerequisite for writing.

When Redis is unreachable the whole thing fails open. Reads show no lock, a claim reports unknown, writes proceed, and a warning lands in the log. I would rather have two users collide once a year than have the system refuse all edits because a cache is down.

Lock changes go out over the existing SignalR hub as an entity change with operations Acquired, Renewed, Released and TakenOver. Renewals are broadcast too, so every open grid shows the correct expiry. Grids show a Locked by column and dim rows held by someone else. The detail form puts the body behind inert and shows a banner, and when the other user releases, the form claims on its own and the banner disappears.

Then it went to production

Everything passed in QA. In production the bulk lookup failed on every single call.

1
2
3
StackExchange.Redis.RedisCommandException:
Multi-key operations must involve a single slot;
keys can use 'hash tags' to help this, i.e. '{/users/12345}/account' and '{/users/12345}/contacts' will always be in the same slot

QA ran against a small Azure Managed Redis instance too, so what was different? Luck. The grid asked for the locks of every row on the page with one MGET. Azure Managed Redis is clustered, and a multi key command is refused when the keys hash to different slots. In QA the few locked rows happened to share a slot often enough that nobody saw the error. In production, with real users and real pages, the first lookup with two locks in different slots blew up, and after that every lookup did.

The single key operations were never affected. Claim, renew and release touch one lock key and one index key, and I had not yet noticed that those two could also land in different slots because the index only arrived a few days later. The fix is the one the error message spells out. Put the tenant in a hash tag.

1
2
lock:{<tenant>}:<type>:<uid>
lockidx:{<tenant>}:<type>

Redis hashes only the part inside the braces, so every lock of a tenant and the index of that tenant sit in one slot. One script may touch both, and one MGET may read many. A unit test now pins the key shape, because this is exactly the sort of thing someone cleans up in a refactor. Keys in the old format were left alone; they expired with their lease.

I had read about hash tags years ago and filed them under things I will never need. Classic Azure Cache for Redis on a Basic or Standard tier is not clustered, so a lot of .NET code written against it has never met this error. Managed Redis is a different animal, and it is the tier Microsoft wants everyone on.

Reading locks by type instead of by row

The slot fix made the lookup work. It did not make it scale. A grid with two hundred rows posted two hundred uids to get back the three locks that existed, and the transport list is much bigger than two hundred rows.

Reads now go by type. Each tenant and entity type has a sorted set whose members are the lock keys and whose scores are the expiry times. Listing is a prune of what has run out, a range of what is live, then one MGET.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
public async Task<IReadOnlyList<EntityLock>> ListAsync(string indexKey, CancellationToken token = default)
{
    var now = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds();
    var batch = Db.CreateBatch();
    var prune = batch.SortedSetRemoveRangeByScoreAsync(indexKey, double.NegativeInfinity, now);
    var live = batch.SortedSetRangeByScoreAsync(indexKey, now, double.PositiveInfinity);
    batch.Execute();
    await prune;
    var members = await live;
    if (members.Length == 0)
        return [];
    return await GetManyAsync(members.Select(m => (string)m!).ToList(), token);
}

A key that expires between the range and the MGET is simply missing from the result, which is the right answer. The index itself carries the TTL of its newest lock, so a tenant with no activity leaves nothing behind. The Mine endpoint walks the per type indexes instead of a KEYS scan, which I should never have written in the first place.

One more thing changed in that commit. RedisCommandException now counts as a store failure. Before, fail open only covered a lost connection. A refused command threw straight through to the user.

What I would tell myself before starting

Test against the clustered tier you deploy to. The emulator and the small instances hide slot errors until real data spreads keys around.

Decide early what a lock does not protect. Writing that automation and drivers are never blocked was the best decision in the design, and it was made in a meeting before any code existed.

Fail open, and log it. A lock service that takes the application down with it has the wrong priorities.

Put the tenant in the hash tag from day one. It costs nothing and it is the difference between one MGET and a loop.

All rights reserved
Built with Hugo
Theme Stack designed by Jimmy