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