Usage limits: token limits refuse, key limits count on the key, Retry-After, Redis Cluster - #81
Merged
Merged
Conversation
…ay when to retry
Four faults let traffic through a configured limit or misreported it:
- Token rules never refused anything. The pre-call check read only
`requests` rules, and the post-call script was all-or-nothing, so a
request that would take a token window past its limit recorded
nothing: the window stopped counting just below its cap. `admit` now
refuses a request once any window, requests or tokens, is full, and
`record` adds what a call used unconditionally. A window can
overshoot by what was in flight when it filled; the next request is
refused.
- Every rule became a user rule. The auth middleware merged a key's
rules into its owner's, replacing the owner's rule of the same
window, and the gateway counted them all on the user's counter: all
of a user's keys shared one, and the console's per-key usage read a
counter nothing wrote. A key's constraints now travel apart from its
owner's and count on the key lineage's counters, and both sets are
checked, so a key's limit narrows its owner's and never widens it.
The MCP gateway gets the same.
- A request a spent budget refused had already been charged by the
request limits. The budget peek, which charges nothing, now runs
first, and every rate limit is checked and charged in one script, so
a refused request leaves every counter as it was.
- `Retry-After` was a fixed 30 s, which made a spent monthly budget
look retryable within the minute. It is now when the limiting window
has room again, or when the budget's UTC period ends, and a budget
refusal adds `x-should-retry: false`, which the OpenAI and Anthropic
SDKs check before retrying a 429 by themselves.
Rewriting the script also fixes a fifth fault. Its reply nested the
per-rule counts in an array, which fred cannot read as `Vec<i64>` once
there are two rules or more. The parse error sent the gateway to run
the script again through EVAL, charging the request twice, and then to
fail open; with `security.rate_limit_fail_closed` it refused every
request. The reply is flat now, and EVAL is retried on NOSCRIPT only.
A counter is now one Redis hash of its 60 buckets rather than 60
string keys, under a key tagged `{user:<id>}`: every counter of one
request shares a Redis Cluster slot, and the script declares every key
it touches instead of building bucket names of its own. Counters under
the old names are abandoned; windows refill.
Also corrects the schema comment that put team budgets in
`budget_caps`, which the CHECK constraint forbids.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uster
A Redis Cluster refuses a script whose keys hash to several slots and a
DEL across slots (CROSSSLOT), and answers SCAN for the node it is sent
to only. Route health declared three keys per script, quotas and the
RPM/TPM limiter two, with no common hash tag; the gateway and MCP
response caches deleted by pattern with one SCAN and multi-key DELs. On
a cluster, circuit breakers never recorded a sample, quotas never
counted, and cache invalidation reached one node at most.
Their keys now carry a hash tag (`{<route_id>}`, `{<quota key>}`), so a
script's keys share a slot, and `redis_keys::delete_matching` scans
every primary and deletes slot by slot. Budget, login-decay, lockout
and permission-cache commands touch one key each and were already safe.
Route health starts fresh on upgrade: the old keys are abandoned, and
the lifetime counters, which never expire, can be deleted by pattern
(CHANGELOG).
`tests/redis_cluster.rs` runs the limit scripts, budgets, route health,
quotas, a pattern delete, config change notices and the gateway itself
against the cluster TEST_REDIS_CLUSTER_URL names, and skips without
one, so CI's single Redis stays enough. The harness takes a Redis URL
override for it. The Helm README says how to point REDIS_URL at a
cluster.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes in the gateway's usage limits, found while comparing them with a design for ThinkWatch Core, and Redis Cluster support for every multi-key Redis operation.
Limits (
fix(limits))tokensrate limits never refused a request, and stopped counting once a request would have crossed the limit. They now refuse once the window's recorded usage reaches the limit, and usage after a call is always recorded.requestslimits, the script's reply could not be parsed: every admitted request was counted twice, and withrate_limit_fail_closedevery request was refused.Retry-Afterwas a fixed 30 s. It now says when the window has room again or when a spent budget's period ends (UTC); spent budgets also sendx-should-retry: false.teamsubject.Redis Cluster (
fix(redis)){user:<id>},{<route_id>},{<quota key>}).redis_keys::delete_matchingscans every primary and deletes slot by slot.redis-cluster://host:port?node=…works end to end (server run against a 3-node Redis 8 cluster); the Helm README and.env.examplesay how.tests/redis_cluster.rsruns againstTEST_REDIS_CLUSTER_URLand skips without it.Operators will notice (CHANGELOG, Read before upgrading): rate-limit windows and route health start empty after the upgrade (budgets are kept); a key's limits now narrow its owner's limits instead of replacing them, so a key given a higher limit than its owner needs the owner's limit raised.
Each bug was reproduced first by an integration test that failed on the old code. Local: fmt, clippy (lib/bins and tests), 736 unit tests, 401 integration tests on Postgres 18, Redis 8 and ClickHouse 26.3, plus the cluster tests against a local cluster.
🤖 Generated with Claude Code