perf(rate_limit): read the counter TTL once per check

Both Lua scripts read the key's TTL twice: once before mutating, to decide whether to EXPIRE, and once after, to return the window's remaining time. The pre-read is redundant. INCRBYFLOAT and SADD both preserve an existing key's TTL and create a missing key with no expiry, so reading the TTL after the mutation gives what the pre-read gave. It still tells apart the two cases the script has to handle: -2 for a missing key, -1 for a key left without an expiry by some earlier bug.

So move the read after the mutation and drop the second one. INCR_SCRIPT goes from four unconditional Redis calls to three, SADD_SCRIPT from five to four. Atomicity is unchanged, TTL-less keys still self-heal, and the {count, ttl} return contract is the same, so the existing specs for the three TTL states of each script pass without modification.

One thing does change shape. On the first write of a window the script now returns the period it just set rather than reading it back from Redis. Same value, and reset_at still derives from it.

How much this is worth

TTL is the most-called command on gprd's redis-cluster-ratelimiting, roughly 115k/s out of 566k/s, since the rack shadow went to 100% on 2026-07-13 and every check spends two of them.

Halving the count only buys about 1% of node CPU though. Command execution is some 18% of process CPU on the busiest primary, and this removes no round trips, so the ceiling on any per-command optimisation here is low. Dropping whole EVALSHA calls is what would matter for capacity, and that is what :skip does (!326 (merged)).

I mention this because the earlier framing of the saving was in command counts, and a 20%-of-all-commands figure reads like a headroom lever when it is not one. Treat this as hygiene.

Verification

Ran the old and new scripts side by side against Redis 8.6.3 across every TTL state, comparing both the return value and the resulting stored TTL:

key state INCR old vs new SADD old vs new
missing ["1", 60] = ["1", 60] [1, 60] = [1, 60]
exists, TTL 42 ["6", 42] = ["6", 42] [3, 42] = [3, 42]
exists, no expiry ["6", 60] = ["6", 60] [3, 60] = [3, 60]

Identical in all six, and the stored TTLs match too. cost=0 against a missing key still allocates it with the rule's period.

INFO commandstats over 20 checks confirms the fan-out: 20 evalsha, 20 ttl (was 40), 20 incrbyfloat, 1 expire.

The design document does not constrain any of this. Its window-semantics row is still an open decision (gitlab-com/gl-infra/production-engineering#28830 (closed)), and reset_at is not part of the documented Result object, so there was no contract to hold to beyond labkit's own README, which this MR updates.

Sequencing

Not on the critical path for enforce. That is waiting on the divergence go/no-go signal in https://gitlab.com/gitlab-com/gl-infra/production-engineering/-/work_items/29362, not on Redis headroom, so there is no rush on this one.

Related to gitlab-com/gl-infra/production-engineering#28807

🤖 beep boop

Edited by Bob Van Landuyt

Merge request reports

Loading
Loading