The error report said a user had made dozens of requests in a twenty-second window. They hadn't. Nothing in the traffic looked like a burst, and the number was too high for any plausible client behaviour.
So we spent a while looking at the client. That's the natural place to look: a retry loop gone wrong, a component mounting twice, something firing on every keystroke. The rate limiter itself never came up as a suspect. It's a counter in Redis with a TTL on it. Redis would just work.
The limiter
A fixed-window rate limiter backed by Redis: one key per thing being limited, holding a request counter. The first request creates the key with a TTL equal to the window, later requests increment it, and once the count passes the limit requests get rejected until the key expires.
The implementation split that across two commands:
count = GET key
if count is missing:
SET key 1 EXAT <now + window> # the only place a TTL is set
if count >= limit:
reject
INCR key
The flaw
INCR on a missing key doesn't fail. It recreates the key with value 1 and no expiry. And the only line that sets a TTL is the create branch, which by definition only runs when the key doesn't exist. So every increment that lands on a missing key mints a counter that will never expire.
Getting there takes a race: GET returns a value, the key expires, and the INCR arrives a moment later.
What you're left with is a counter that only ever goes up. It climbs by one per request, forever, and the moment it crosses the limit that user is rejected on that endpoint permanently.
Finding it
The audit is read-only and pleasantly boring: SCAN the key prefix, pipeline a TTL for each key, count the ones that come back -1. That's Redis for "this key exists and has no expiry", as distinct from -2, which means it doesn't exist.
The fix
Make the increment and the expiry one atomic unit, and use the increment's return value instead of a separate read:
MULTI
INCR key
EXPIRE key <window> NX
EXEC
EXPIRE ... NX sets the expiry only if the key currently has none. One flag, three useful behaviours:
- Key was just created by the
INCR, so it has no TTL, so it gets the window. - Key is mid-window and already has a TTL, so this is a no-op. Traffic doesn't extend the window, which is what keeps this a fixed window rather than an accidental sliding one.
- Key already lost its expiry, so it gets one back.
That third case is the nice part. The deploy repairs leaked keys as they're touched. Anything still in active use heals itself the next time someone hits the endpoint, and there's no migration to write for it.
Two write paths, one TTL
The bug needs two things: a key that's meant to expire, and more than one path that can create it, where at least one of those paths doesn't set the expiry. Plenty of Redis commands quietly create a key when it's missing, and INCR is one of them. I did not have that on the list of things INCR does.
So if a key is supposed to expire, every command that can create it has to set the expiry too, in the same atomic unit. MULTI for the atomicity, NX so that setting it doesn't clobber a window that's already running.